[RFC PATCH 0/7] mmc: Add OMAP SDHCI driver

[RFC PATCH 0/7] mmc: Add OMAP SDHCI driver

[Kishon Vijay Abraham I]

This is the first step in deprecating omap_hsmmc driver completely
and moving to sdhci-omap driver which uses the sdhci library.

This series adds 3 new quirks to sdhci library in order to support
MMC in OMAP.
  *) to avoid CRC stripping in MMC_RSP_136
  *) to indicate controller supports ADMA2
  *) to indicate broken POWER_CONTROL (POWER_CONTROL in TI's SOC
     controls IO voltage instead of Vdd).

Apart from the above mentioned quirks, sdhci-omap has it's own callbacks
to set_clock (clock divider programming is different from generic sdhci)
, set_bus_width, set_bus_mode and platform_send_init_74_clocks. These
callback functions are implemented based on omap_hsmmc driver.

The sdhci-omap driver supports only the high speed mode and UHS/HS200
mode will be added in a later series.

It has been tested only in boards having DRA7 SoCs like dra7-evm, dra72-evm,
am571x-idk, am572x-idk, am57xx-evm. (Tested only eMMC and SD.
SDIO support will be added later). The plan is to fully convert DRA7
SoC to use SDHCI driver and then convert other legacy platforms to use
SDHCI.

dts patches will be sent as a separate series.

I've also pushed the entire series along with dependent dt patches @
https://github.com/kishon/linux-wip.git sdhci_omap_v1 (in case someone
wants to test)

Kishon Vijay Abraham I (7):
  mmc: sdhci: Add quirk to indicate no CRC stripping in MMC_RSP_136
  mmc: sdhci: Add quirk to indicate controller supports ADMA2
  mmc: sdhci: Add callback to set bus mode
  mmc: sdhci: Add quirk to indicate broken POWER_CONTROL
  dt-bindings: ti-omap-hsmmc: Document new compatible for sdhci omap
  mmc: sdhci-omap: Add OMAP SDHCI driver
  MAINTAINERS: Add TI OMAP SDHCI Maintainer

 .../devicetree/bindings/mmc/ti-omap-hsmmc.txt      |   1 +
 MAINTAINERS                                        |   6 +
 drivers/mmc/host/Kconfig                           |  12 +
 drivers/mmc/host/Makefile                          |   1 +
 drivers/mmc/host/sdhci-omap.c                      | 593 +++++++++++++++++++++
 drivers/mmc/host/sdhci.c                           |  39 +-
 drivers/mmc/host/sdhci.h                           |   7 +
 7 files changed, 650 insertions(+), 9 deletions(-)
 create mode 100644 drivers/mmc/host/sdhci-omap.c

-- 
2.11.0

[RFC PATCH 1/7] mmc: sdhci: Add quirk to indicate no CRC stripping in MMC_RSP_136

[Kishon Vijay Abraham I]

TI's implementation of sdhci controller used in DRA7 SoC's doesn't
strip CRC in responses with length 136 bits. Add quirk to indicate
the controller does not strip CRC in MMC_RSP_136. If this quirk is
set sdhci library shouldn't shift the response present in
SDHCI_RESPONSE register.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/mmc/host/sdhci.c | 27 +++++++++++++++++++--------
 drivers/mmc/host/sdhci.h |  2 ++
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index ecd0d4350e8a..ece3751d2a25 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1182,14 +1182,25 @@ static void sdhci_finish_command(struct sdhci_host *host)
 
 	if (cmd->flags & MMC_RSP_PRESENT) {
 		if (cmd->flags & MMC_RSP_136) {
-			/* CRC is stripped so we need to do some shifting. */
-			for (i = 0;i < 4;i++) {
-				cmd->resp[i] = sdhci_readl(host,
-					SDHCI_RESPONSE + (3-i)*4) << 8;
-				if (i != 3)
-					cmd->resp[i] |=
-						sdhci_readb(host,
-						SDHCI_RESPONSE + (3-i)*4-1);
+			if (!(host->quirks2 & SDHCI_QUIRK2_NO_CRC_STRIPPING)) {
+				/*
+				 * CRC is stripped so we need to do some
+				 * shifting.
+				 */
+				for (i = 0; i < 4; i++) {
+					cmd->resp[i] =
+						sdhci_readl(host, SDHCI_RESPONSE
+							    + (3 - i) * 4) << 8;
+					if (i != 3)
+						cmd->resp[i] |=
+						sdhci_readb(host, SDHCI_RESPONSE
+							    + (3 - i) * 4 - 1);
+				}
+			} else {
+				for (i = 0; i < 4; i++)
+					cmd->resp[i] =
+					sdhci_readl(host, SDHCI_RESPONSE +
+						    (3 - i) * 4);
 			}
 		} else {
 			cmd->resp[0] = sdhci_readl(host, SDHCI_RESPONSE);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 0469fa191493..6905131f603d 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -435,6 +435,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK2_ACMD23_BROKEN			(1<<14)
 /* Broken Clock divider zero in controller */
 #define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN		(1<<15)
+/* Controller does not have CRC stripped in Command Response */
+#define SDHCI_QUIRK2_NO_CRC_STRIPPING			(1<<16)
 
 	int irq;		/* Device IRQ */
 	void __iomem *ioaddr;	/* Mapped address */
-- 
2.11.0

[RFC PATCH 2/7] mmc: sdhci: Add quirk to indicate controller supports ADMA2

[Kishon Vijay Abraham I]

TI's implementation of sdhci controller used in DRA7 SoC's doesn't
have SDHCI_CAN_DO_ADMA2 set in CAPA register though it supports
ADMA2. Add quirk to support using ADMA2 even if the controller reports
incorrect capability in CAPA.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/mmc/host/sdhci.c | 3 ++-
 drivers/mmc/host/sdhci.h | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index ece3751d2a25..fff0baadbc3e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -3264,7 +3264,8 @@ int sdhci_setup_host(struct sdhci_host *host)
 	}
 
 	if ((host->version >= SDHCI_SPEC_200) &&
-		(host->caps & SDHCI_CAN_DO_ADMA2))
+		((host->caps & SDHCI_CAN_DO_ADMA2) ||
+		(host->quirks2 & SDHCI_QUIRK2_FORCE_ADMA)))
 		host->flags |= SDHCI_USE_ADMA;
 
 	if ((host->quirks & SDHCI_QUIRK_BROKEN_ADMA) &&
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 6905131f603d..d778034e324d 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -437,6 +437,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN		(1<<15)
 /* Controller does not have CRC stripped in Command Response */
 #define SDHCI_QUIRK2_NO_CRC_STRIPPING			(1<<16)
+/* Controller has bad caps bits, but really supports DMA */
+#define SDHCI_QUIRK2_FORCE_ADMA				(1<<17)
 
 	int irq;		/* Device IRQ */
 	void __iomem *ioaddr;	/* Mapped address */
-- 
2.11.0

[RFC PATCH 3/7] mmc: sdhci: Add callback to set bus mode

[Kishon Vijay Abraham I]

Add callback to set bus mode in sdhci library so that the
controller driver can perform any bus mode specific configurations
in the callback function.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/mmc/host/sdhci.c | 3 +++
 drivers/mmc/host/sdhci.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index fff0baadbc3e..d5050ec42481 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1747,6 +1747,9 @@ void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	if (host->quirks & SDHCI_QUIRK_RESET_CMD_DATA_ON_IOS)
 		sdhci_do_reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA);
 
+	if (host->ops->set_bus_mode)
+		host->ops->set_bus_mode(host, ios->bus_mode);
+
 	mmiowb();
 }
 EXPORT_SYMBOL_GPL(sdhci_set_ios);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index d778034e324d..39c7a2723906 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -583,6 +583,7 @@ struct sdhci_ops {
 	void    (*adma_workaround)(struct sdhci_host *host, u32 intmask);
 	void    (*card_event)(struct sdhci_host *host);
 	void	(*voltage_switch)(struct sdhci_host *host);
+	void	(*set_bus_mode)(struct sdhci_host *host, unsigned int mode);
 };
 
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
-- 
2.11.0

[RFC PATCH 4/7] mmc: sdhci: Add quirk to indicate broken POWER_CONTROL

[Kishon Vijay Abraham I]

TI's implementation of sdhci controller used in DRA7 SoC's uses
POWER_CONTROL register for configuring IO voltage and not
for core voltage (vdd) which is it's intended use. Add a quirk
to indicate broken POWER_CONTROL register.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/mmc/host/sdhci.c | 6 ++++++
 drivers/mmc/host/sdhci.h | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index d5050ec42481..a7465a56e6b5 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1419,6 +1419,9 @@ static void sdhci_set_power_reg(struct sdhci_host *host, unsigned char mode,
 
 	mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
 
+	if (host->quirks2 & SDHCI_QUIRK2_BROKEN_POWER_CONTROL)
+		return;
+
 	if (mode != MMC_POWER_OFF)
 		sdhci_writeb(host, SDHCI_POWER_ON, SDHCI_POWER_CONTROL);
 	else
@@ -1430,6 +1433,9 @@ void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
 {
 	u8 pwr = 0;
 
+	if (host->quirks2 & SDHCI_QUIRK2_BROKEN_POWER_CONTROL)
+		return;
+
 	if (mode != MMC_POWER_OFF) {
 		switch (1 << vdd) {
 		case MMC_VDD_165_195:
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 39c7a2723906..ba94cca7c892 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -439,6 +439,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK2_NO_CRC_STRIPPING			(1<<16)
 /* Controller has bad caps bits, but really supports DMA */
 #define SDHCI_QUIRK2_FORCE_ADMA				(1<<17)
+/* Controller uses POWER_CONTROL for managing IO voltage and not core voltage */
+#define SDHCI_QUIRK2_BROKEN_POWER_CONTROL		(1<<18)
 
 	int irq;		/* Device IRQ */
 	void __iomem *ioaddr;	/* Mapped address */
-- 
2.11.0

[RFC PATCH 5/7] dt-bindings: ti-omap-hsmmc: Document new compatible for sdhci omap

[Kishon Vijay Abraham I]

Document the new compatible string "ti,dra7-sdhci" to be used for
MMC controllers in DRA7 and DRA72 SoCs.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
index 0e026c151c1c..db80fdfd05d7 100644
--- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
+++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
@@ -13,6 +13,7 @@ Required properties:
  Should be "ti,omap3-pre-es3-hsmmc" for OMAP3 controllers pre ES3.0
  Should be "ti,omap4-hsmmc", for OMAP4 controllers
  Should be "ti,am33xx-hsmmc", for AM335x controllers
+ Should be "ti,dra7-sdhci" for DRA7 and DRA72 controllers
 - ti,hwmods: Must be "mmc<n>", n is controller instance starting 1
 
 Optional properties:
-- 
2.11.0

[RFC PATCH 6/7] mmc: sdhci-omap: Add OMAP SDHCI driver

[Kishon Vijay Abraham I]

Create a new sdhci-omap driver to configure the eMMC/SD/SDIO controller
in TI's OMAP SoCs making use of the SDHCI core library. For OMAP specific
configurations, populate sdhci_ops with OMAP specific callbacks and use
SDHCI quirks.
Enable only high speed mode for both SD and eMMC here and add other
UHS mode support later.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/mmc/host/Kconfig      |  12 +
 drivers/mmc/host/Makefile     |   1 +
 drivers/mmc/host/sdhci-omap.c | 593 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 606 insertions(+)
 create mode 100644 drivers/mmc/host/sdhci-omap.c

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 5755b69f2f72..9aa7e5ce43ba 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -871,3 +871,15 @@ config MMC_SDHCI_XENON
 	  This selects Marvell Xenon eMMC/SD/SDIO SDHCI.
 	  If you have a controller with this interface, say Y or M here.
 	  If unsure, say N.
+
+config MMC_SDHCI_OMAP
+	tristate "TI SDHCI Controller Support"
+	depends on MMC_SDHCI_PLTFM
+	help
+	  This selects the Secure Digital Host Controller Interface (SDHCI)
+	  support present in TI's DRA7 SOCs. The controller supports
+	  SD/MMC/SDIO devices.
+
+	  If you have a controller with this interface, say Y or M here.
+
+	  If unsure, say N.
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 4d4547116311..30fb8b0fd0e1 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_MMC_SDHCI_MSM)		+= sdhci-msm.o
 obj-$(CONFIG_MMC_SDHCI_ST)		+= sdhci-st.o
 obj-$(CONFIG_MMC_SDHCI_MICROCHIP_PIC32)	+= sdhci-pic32.o
 obj-$(CONFIG_MMC_SDHCI_BRCMSTB)		+= sdhci-brcmstb.o
+obj-$(CONFIG_MMC_SDHCI_OMAP)		+= sdhci-omap.o
 
 ifeq ($(CONFIG_CB710_DEBUG),y)
 	CFLAGS-cb710-mmc	+= -DDEBUG
diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
new file mode 100644
index 000000000000..c067c428a0cd
--- /dev/null
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -0,0 +1,593 @@
+/*
+ * SDHCI Controller driver for TI's OMAP SoCs
+ *
+ * Copyright (C) 2016-2017 Texas Instruments Incorporated - http://www.ti.com
+ *
+ * Authors: Kishon Vijay Abraham I <kishon@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/delay.h>
+#include <linux/mmc/slot-gpio.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+
+#include "sdhci-pltfm.h"
+
+#define SDHCI_OMAP_CON		0x12c
+#define CON_DW8			BIT(5)
+#define CON_DMA_MASTER		BIT(20)
+#define CON_INIT		BIT(1)
+#define CON_OD			BIT(0)
+
+#define SDHCI_OMAP_CMD		0x20c
+
+#define SDHCI_OMAP_HCTL		0x228
+#define HCTL_SDBP		BIT(8)
+#define HCTL_SDVS_SHIFT		9
+#define HCTL_SDVS_MASK		(0x7 << HCTL_SDVS_SHIFT)
+#define HCTL_SDVS_33		(0x7 << HCTL_SDVS_SHIFT)
+#define HCTL_SDVS_30		(0x6 << HCTL_SDVS_SHIFT)
+#define HCTL_SDVS_18		(0x5 << HCTL_SDVS_SHIFT)
+
+#define SDHCI_OMAP_SYSCTL	0x22c
+#define SYSCTL_CEN		BIT(2)
+#define SYSCTL_CLKD_SHIFT	6
+#define SYSCTL_CLKD_MASK	0x3ff
+
+#define SDHCI_OMAP_STAT		0x230
+
+#define SDHCI_OMAP_IE		0x234
+#define INT_CC_EN		BIT(0)
+
+#define SDHCI_OMAP_AC12		0x23c
+#define AC12_V1V8_SIGEN		BIT(19)
+
+#define SDHCI_OMAP_CAPA		0x240
+#define CAPA_VS33		BIT(24)
+#define CAPA_VS30		BIT(25)
+#define CAPA_VS18		BIT(26)
+
+#define MMC_TIMEOUT		1		/* 1 msec */
+
+#define SYSCTL_CLKD_MAX		0x3FF
+
+#define IOV_1V8			1800000		/* 180000 uV */
+#define IOV_3V0			3000000		/* 300000 uV */
+#define IOV_3V3			3300000		/* 330000 uV */
+
+struct sdhci_omap_data {
+	u32 offset;
+};
+
+struct sdhci_omap_host {
+	void __iomem		*base;
+	struct device		*dev;
+	struct	regulator	*pbias;
+	bool			pbias_enabled;
+	struct sdhci_host	*host;
+	unsigned int		flags;
+
+#define SDHCI_OMAP_SUPPORTS_DUAL_VOLT	BIT(0)
+#define SDHCI_OMAP_NO_1_8_V		BIT(1)
+
+	u8			power_mode;
+	bool			io_3_3v_support;
+};
+
+static inline u32 sdhci_omap_readl(struct sdhci_omap_host *host,
+				   unsigned int offset)
+{
+	return readl(host->base + offset);
+}
+
+static inline void sdhci_omap_writel(struct sdhci_omap_host *host,
+				     unsigned int offset, u32 data)
+{
+	writel(data, host->base + offset);
+}
+
+static int sdhci_omap_set_pbias(struct sdhci_omap_host *omap_host,
+				bool power_on, unsigned int iov)
+{
+	int ret;
+	struct device *dev = omap_host->dev;
+
+	if (IS_ERR(omap_host->pbias))
+		return 0;
+
+	if (power_on) {
+		ret = regulator_set_voltage(omap_host->pbias, iov, iov);
+		if (ret) {
+			dev_err(dev, "pbias set voltage failed\n");
+			return ret;
+		}
+
+		if (omap_host->pbias_enabled)
+			return 0;
+
+		ret = regulator_enable(omap_host->pbias);
+		if (ret) {
+			dev_err(dev, "pbias reg enable fail\n");
+			return ret;
+		}
+
+		omap_host->pbias_enabled = true;
+	} else {
+		if (!omap_host->pbias_enabled)
+			return 0;
+
+		ret = regulator_disable(omap_host->pbias);
+		if (ret) {
+			dev_err(dev, "pbias reg disable fail\n");
+			return ret;
+		}
+		omap_host->pbias_enabled = false;
+	}
+
+	return 0;
+}
+
+static int sdhci_omap_enable_iov(struct sdhci_omap_host *omap_host,
+				 unsigned int iov)
+{
+	int ret;
+	struct sdhci_host *host = omap_host->host;
+	struct mmc_host *mmc = host->mmc;
+
+	ret = sdhci_omap_set_pbias(omap_host, false, 0);
+	if (ret)
+		return ret;
+
+	if (!IS_ERR(mmc->supply.vqmmc)) {
+		ret = regulator_set_voltage(mmc->supply.vqmmc, iov, iov);
+		if (ret) {
+			dev_err(mmc_dev(mmc), "vqmmc set voltage failed\n");
+			return ret;
+		}
+	}
+
+	ret = sdhci_omap_set_pbias(omap_host, true, iov);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void sdhci_omap_conf_bus_power(struct sdhci_omap_host *omap_host,
+				      unsigned char signal_voltage)
+{
+	u32 reg;
+	ktime_t timeout;
+
+	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_HCTL);
+	reg &= ~HCTL_SDVS_MASK;
+
+	if (signal_voltage == MMC_SIGNAL_VOLTAGE_180)
+		reg |= HCTL_SDVS_18;
+	else if (omap_host->io_3_3v_support)
+		reg |= HCTL_SDVS_33;
+	else
+		reg |= HCTL_SDVS_30;
+
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_HCTL, reg);
+
+	reg |= HCTL_SDBP;
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_HCTL, reg);
+
+	/* wait 1ms */
+	timeout = ktime_add_ms(ktime_get(), MMC_TIMEOUT);
+	while (!(sdhci_omap_readl(omap_host, SDHCI_OMAP_HCTL) & HCTL_SDBP)) {
+		if (WARN_ON(ktime_after(ktime_get(), timeout)))
+			return;
+		usleep_range(5, 10);
+	}
+}
+
+static int sdhci_omap_start_signal_voltage_switch(struct mmc_host *mmc,
+						  struct mmc_ios *ios)
+{
+	u32 reg;
+	int ret;
+	unsigned int iov;
+	struct sdhci_host *host = mmc_priv(mmc);
+	struct sdhci_pltfm_host *pltfm_host;
+	struct sdhci_omap_host *omap_host;
+	struct device *dev;
+
+	pltfm_host = sdhci_priv(host);
+	omap_host = sdhci_pltfm_priv(pltfm_host);
+	dev = omap_host->dev;
+
+	if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
+		reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CAPA);
+		if (!(reg & (CAPA_VS33 | CAPA_VS30)))
+			return -EOPNOTSUPP;
+
+		sdhci_omap_conf_bus_power(omap_host, ios->signal_voltage);
+
+		reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_AC12);
+		reg &= ~AC12_V1V8_SIGEN;
+		sdhci_omap_writel(omap_host, SDHCI_OMAP_AC12, reg);
+
+		if (omap_host->io_3_3v_support)
+			iov = IOV_3V3;
+		else
+			iov = IOV_3V0;
+	} else if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_180) {
+		reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CAPA);
+		if (!(reg & CAPA_VS18))
+			return -EOPNOTSUPP;
+
+		sdhci_omap_conf_bus_power(omap_host, ios->signal_voltage);
+
+		reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_AC12);
+		reg |= AC12_V1V8_SIGEN;
+		sdhci_omap_writel(omap_host, SDHCI_OMAP_AC12, reg);
+
+		iov = IOV_1V8;
+	} else {
+		return -EOPNOTSUPP;
+	}
+
+	ret = sdhci_omap_enable_iov(omap_host, iov);
+	if (ret) {
+		dev_err(dev, "failed to switch IO voltage to %dmV\n", iov);
+		return ret;
+	}
+
+	dev_dbg(dev, "IO voltage switched to %dmV\n", iov);
+	return 0;
+}
+
+static void sdhci_omap_set_capabilities(struct sdhci_omap_host *omap_host)
+{
+	u32 reg;
+
+	/* voltage capabilities might be set by boot loader, clear it */
+	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CAPA);
+	reg &= ~(CAPA_VS18 | CAPA_VS30 | CAPA_VS33);
+
+	if (omap_host->flags & SDHCI_OMAP_SUPPORTS_DUAL_VOLT) {
+		if (omap_host->io_3_3v_support)
+			reg |= (CAPA_VS33 | CAPA_VS18);
+		else
+			reg |= (CAPA_VS30 | CAPA_VS18);
+	} else if (omap_host->flags & SDHCI_OMAP_NO_1_8_V) {
+		if (omap_host->io_3_3v_support)
+			reg |= CAPA_VS33;
+		else
+			reg |= CAPA_VS30;
+	} else {
+		reg |= CAPA_VS18;
+	}
+
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_CAPA, reg);
+}
+
+static void sdhci_omap_set_bus_mode(struct sdhci_host *host, unsigned int mode)
+{
+	u32 reg;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
+
+	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CON);
+	if (mode == MMC_BUSMODE_OPENDRAIN)
+		reg |= CON_OD;
+	else
+		reg &= ~CON_OD;
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_CON, reg);
+}
+
+static void sdhci_omap_set_bus_width(struct sdhci_host *host, int width)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
+	u32 reg;
+
+	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CON);
+	if (width == MMC_BUS_WIDTH_8)
+		reg |= CON_DW8;
+	else
+		reg &= ~CON_DW8;
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_CON, reg);
+
+	sdhci_set_bus_width(host, width);
+}
+
+static void sdhci_omap_init_74_clocks(struct sdhci_host *host, u8 power_mode)
+{
+	u32 reg;
+	ktime_t timeout;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
+
+	if (omap_host->power_mode == power_mode)
+		return;
+
+	if (power_mode != MMC_POWER_ON)
+		return;
+
+	disable_irq(host->irq);
+
+	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CON);
+	reg |= CON_INIT;
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_CON, reg);
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_CMD, 0x0);
+
+	/* wait 1ms */
+	timeout = ktime_add_ms(ktime_get(), MMC_TIMEOUT);
+	while (!(sdhci_omap_readl(omap_host, SDHCI_OMAP_STAT) & INT_CC_EN)) {
+		if (WARN_ON(ktime_after(ktime_get(), timeout)))
+			return;
+		usleep_range(5, 10);
+	}
+
+	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CON);
+	reg &= ~CON_INIT;
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_CON, reg);
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_STAT, INT_CC_EN);
+
+	enable_irq(host->irq);
+
+	omap_host->power_mode = power_mode;
+}
+
+static u16 sdhci_omap_calc_divisor(struct sdhci_pltfm_host *host,
+				   unsigned int clock)
+{
+	u16 dsor;
+
+	dsor = DIV_ROUND_UP(clk_get_rate(host->clk), clock);
+	if (dsor > SYSCTL_CLKD_MAX)
+		dsor = SYSCTL_CLKD_MAX;
+
+	return dsor;
+}
+
+static void sdhci_omap_start_clock(struct sdhci_omap_host *omap_host)
+{
+	u32 reg;
+
+	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_SYSCTL);
+	reg |= SYSCTL_CEN;
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_SYSCTL, reg);
+}
+
+static void sdhci_omap_stop_clock(struct sdhci_omap_host *omap_host)
+{
+	u32 reg;
+
+	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_SYSCTL);
+	reg &= ~SYSCTL_CEN;
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_SYSCTL, reg);
+}
+
+static void sdhci_omap_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
+	unsigned long clkdiv;
+
+	if (!clock)
+		return;
+
+	sdhci_omap_stop_clock(omap_host);
+
+	clkdiv = sdhci_omap_calc_divisor(pltfm_host, clock);
+	clkdiv = (clkdiv & SYSCTL_CLKD_MASK) << SYSCTL_CLKD_SHIFT;
+	sdhci_enable_clk(host, clkdiv);
+
+	sdhci_omap_start_clock(omap_host);
+}
+
+static int sdhci_omap_enable_dma(struct sdhci_host *host)
+{
+	u32 reg;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
+
+	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CON);
+	reg |= CON_DMA_MASTER;
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_CON, reg);
+
+	return 0;
+}
+
+unsigned int sdhci_omap_get_min_clock(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+
+	return clk_get_rate(pltfm_host->clk) / SYSCTL_CLKD_MAX;
+}
+
+static struct sdhci_ops sdhci_omap_ops = {
+	.set_clock = sdhci_omap_set_clock,
+	.enable_dma = sdhci_omap_enable_dma,
+	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
+	.get_min_clock = sdhci_omap_get_min_clock,
+	.set_bus_width = sdhci_omap_set_bus_width,
+	.platform_send_init_74_clocks = sdhci_omap_init_74_clocks,
+	.reset = sdhci_reset,
+	.set_uhs_signaling = sdhci_set_uhs_signaling,
+	.set_bus_mode = sdhci_omap_set_bus_mode,
+};
+
+void sdhci_omap_get_of_property(struct sdhci_omap_host *omap_host)
+{
+	struct device *dev = omap_host->dev;
+	struct sdhci_host *host = omap_host->host;
+	struct mmc_host *mmc = host->mmc;
+
+	if (device_property_read_bool(dev, "ti,dual-volt"))
+		omap_host->flags |= SDHCI_OMAP_SUPPORTS_DUAL_VOLT;
+
+	if (device_property_read_bool(dev, "no-1-8-v"))
+		omap_host->flags |= SDHCI_OMAP_NO_1_8_V;
+
+	if (device_property_read_bool(dev, "ti,non-removable"))
+		mmc->caps |= MMC_CAP_NONREMOVABLE;
+}
+
+static const struct sdhci_pltfm_data sdhci_omap_pdata = {
+	.quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION |
+		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
+		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
+		  SDHCI_QUIRK_NO_HISPD_BIT |
+		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
+	.quirks2 = SDHCI_QUIRK2_NO_1_8_V |
+		   SDHCI_QUIRK2_ACMD23_BROKEN |
+		   SDHCI_QUIRK2_NO_CRC_STRIPPING |
+		   SDHCI_QUIRK2_FORCE_ADMA |
+		   SDHCI_QUIRK2_BROKEN_POWER_CONTROL,
+	.ops = &sdhci_omap_ops,
+};
+
+static const struct sdhci_omap_data dra7_data = {
+	.offset = 0x200,
+};
+
+static const struct of_device_id omap_sdhci_match[] = {
+	{ .compatible = "ti,dra7-sdhci", .data = &dra7_data },
+	{},
+};
+MODULE_DEVICE_TABLE(of, omap_sdhci_match);
+
+static int sdhci_omap_probe(struct platform_device *pdev)
+{
+	int ret;
+	u32 offset;
+	struct device *dev = &pdev->dev;
+	struct sdhci_host *host;
+	struct sdhci_pltfm_host *pltfm_host;
+	struct sdhci_omap_host *omap_host;
+	struct mmc_host *mmc;
+	const struct of_device_id *match;
+	struct sdhci_omap_data *data;
+
+	match = of_match_device(omap_sdhci_match, dev);
+	if (!match)
+		return -EINVAL;
+
+	data = (struct sdhci_omap_data *)match->data;
+	if (!data) {
+		dev_err(dev, "no sdhci omap data\n");
+		return -EINVAL;
+	}
+
+	offset = data->offset;
+
+	host = sdhci_pltfm_init(pdev, &sdhci_omap_pdata,
+				sizeof(*omap_host));
+	if (IS_ERR(host)) {
+		dev_err(dev, "Failed sdhci_pltfm_init\n");
+		return PTR_ERR(host);
+	}
+
+	pltfm_host = sdhci_priv(host);
+	omap_host = sdhci_pltfm_priv(pltfm_host);
+	omap_host->host = host;
+	omap_host->base = host->ioaddr;
+	omap_host->dev = dev;
+	sdhci_omap_get_of_property(omap_host);
+
+	host->ioaddr += offset;
+
+	mmc = host->mmc;
+	ret = mmc_of_parse(mmc);
+	if (ret)
+		goto err_of_parse;
+
+	pltfm_host->clk = devm_clk_get(dev, "fck");
+	if (IS_ERR(pltfm_host->clk)) {
+		ret = PTR_ERR(pltfm_host->clk);
+		goto err_of_parse;
+	}
+
+	ret = clk_set_rate(pltfm_host->clk, mmc->f_max);
+	if (ret) {
+		dev_err(dev, "failed to set clock to %d\n", mmc->f_max);
+		goto err_of_parse;
+	}
+
+	omap_host->pbias = devm_regulator_get_optional(dev, "pbias");
+	if (IS_ERR(omap_host->pbias)) {
+		ret = PTR_ERR(omap_host->pbias);
+		if (ret != -ENODEV) {
+			dev_err(dev, "CONFIG_REGULATOR_PBIAS not enabled??\n");
+			return ret;
+		}
+		dev_dbg(dev, "unable to get pbias regulator %ld\n",
+			PTR_ERR(omap_host->pbias));
+	} else {
+		if (regulator_is_supported_voltage(omap_host->pbias, IOV_3V3,
+						   IOV_3V3))
+			omap_host->io_3_3v_support = true;
+		dev_vdbg(dev, "Max PBIAS support Voltage: %dmv\n",
+			 omap_host->io_3_3v_support ? IOV_3V3 : IOV_3V0);
+	}
+	omap_host->pbias_enabled = false;
+
+	pm_runtime_enable(dev);
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		dev_err(dev, "pm_runtime_get_sync failed\n");
+		goto err_get_sync;
+	}
+
+	sdhci_omap_set_capabilities(omap_host);
+
+	host->mmc_host_ops.get_ro = mmc_gpio_get_ro;
+	host->mmc_host_ops.start_signal_voltage_switch =
+					sdhci_omap_start_signal_voltage_switch;
+
+	ret = sdhci_add_host(host);
+	if (ret)
+		goto err_get_sync;
+
+	return 0;
+
+err_get_sync:
+	pm_runtime_put_sync(dev);
+	pm_runtime_disable(dev);
+
+err_of_parse:
+	sdhci_pltfm_free(pdev);
+	return ret;
+}
+
+static int sdhci_omap_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct sdhci_host *host = platform_get_drvdata(pdev);
+
+	sdhci_remove_host(host, true);
+	pm_runtime_put_sync(dev);
+	pm_runtime_disable(dev);
+	sdhci_pltfm_free(pdev);
+
+	return 0;
+}
+
+static struct platform_driver sdhci_omap_driver = {
+	.probe = sdhci_omap_probe,
+	.remove = sdhci_omap_remove,
+	.driver = {
+		   .name = "sdhci-omap",
+		   .of_match_table = of_match_ptr(omap_sdhci_match),
+		  },
+};
+
+module_platform_driver(sdhci_omap_driver);
+
+MODULE_DESCRIPTION("SDHCI driver for OMAP SoCs");
+MODULE_AUTHOR("Texas Instruments Inc.");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("OMAP SDHCI Driver");
-- 
2.11.0

[RFC PATCH 7/7] MAINTAINERS: Add TI OMAP SDHCI Maintainer

[Kishon Vijay Abraham I]

Add Maintainer for the TI OMAP SDHCI driver.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f66488dfdbc9..10c9dde27c0a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11769,6 +11769,12 @@ L:	linux-mmc@vger.kernel.org
 S:	Maintained
 F:	drivers/mmc/host/sdhci-spear.c
 
+SECURE DIGITAL HOST CONTROLLER INTERFACE (SDHCI) TI OMAP DRIVER
+M:	Kishon Vijay Abraham I <kishon@ti.com>
+L:	linux-mmc@vger.kernel.org
+S:	Maintained
+F:	drivers/mmc/host/sdhci-omap.c
+
 SECURE ENCRYPTING DEVICE (SED) OPAL DRIVER
 M:	Scott Bauer <scott.bauer@intel.com>
 M:	Jonathan Derrick <jonathan.derrick@intel.com>
-- 
2.11.0

Re: [RFC PATCH 5/7] dt-bindings: ti-omap-hsmmc: Document new compatible for sdhci omap

[Tony Lindgren]

* Kishon Vijay Abraham I <kishon@ti.com> [170807 09:03]:
> Document the new compatible string "ti,dra7-sdhci" to be used for
> MMC controllers in DRA7 and DRA72 SoCs.

I wonder if this should really be documented for sdhci
instead of ti-omap-hsmmc.txt?

I agree it's better to use sdhci compatible and keep the
hsmmc compatible around as the sdhci is still missing features
for runtime PM and context save and restore etc.

Regards,

Tony


> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> index 0e026c151c1c..db80fdfd05d7 100644
> --- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> @@ -13,6 +13,7 @@ Required properties:
>   Should be "ti,omap3-pre-es3-hsmmc" for OMAP3 controllers pre ES3.0
>   Should be "ti,omap4-hsmmc", for OMAP4 controllers
>   Should be "ti,am33xx-hsmmc", for AM335x controllers
> + Should be "ti,dra7-sdhci" for DRA7 and DRA72 controllers
>  - ti,hwmods: Must be "mmc<n>", n is controller instance starting 1
>  
>  Optional properties:
> -- 
> 2.11.0
> 

Re: [RFC PATCH 1/7] mmc: sdhci: Add quirk to indicate no CRC stripping in MMC_RSP_136

[Adrian Hunter]

On 07/08/17 19:01, Kishon Vijay Abraham I wrote:
> TI's implementation of sdhci controller used in DRA7 SoC's doesn't
> strip CRC in responses with length 136 bits. Add quirk to indicate
> the controller does not strip CRC in MMC_RSP_136. If this quirk is
> set sdhci library shouldn't shift the response present in
> SDHCI_RESPONSE register.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/mmc/host/sdhci.c | 27 +++++++++++++++++++--------
>  drivers/mmc/host/sdhci.h |  2 ++
>  2 files changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index ecd0d4350e8a..ece3751d2a25 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1182,14 +1182,25 @@ static void sdhci_finish_command(struct sdhci_host *host)
>  
>  	if (cmd->flags & MMC_RSP_PRESENT) {
>  		if (cmd->flags & MMC_RSP_136) {
> -			/* CRC is stripped so we need to do some shifting. */
> -			for (i = 0;i < 4;i++) {
> -				cmd->resp[i] = sdhci_readl(host,
> -					SDHCI_RESPONSE + (3-i)*4) << 8;
> -				if (i != 3)
> -					cmd->resp[i] |=
> -						sdhci_readb(host,
> -						SDHCI_RESPONSE + (3-i)*4-1);
> +			if (!(host->quirks2 & SDHCI_QUIRK2_NO_CRC_STRIPPING)) {

This is about the 136-bit response so let's put that in the quirk name.  How about SDHCI_QUIRK2_RSP_136_HAS_CRC

> +				/*
> +				 * CRC is stripped so we need to do some
> +				 * shifting.
> +				 */
> +				for (i = 0; i < 4; i++) {
> +					cmd->resp[i] =
> +						sdhci_readl(host, SDHCI_RESPONSE
> +							    + (3 - i) * 4) << 8;
> +					if (i != 3)
> +						cmd->resp[i] |=
> +						sdhci_readb(host, SDHCI_RESPONSE
> +							    + (3 - i) * 4 - 1);
> +				}
> +			} else {
> +				for (i = 0; i < 4; i++)
> +					cmd->resp[i] =
> +					sdhci_readl(host, SDHCI_RESPONSE +
> +						    (3 - i) * 4);
>  			}

This is all very jammed up against the 80 column margin.  Please make a new patch to separate it into a new function sdhci_read_rsp_136() and then another patch to add the quirk.
i.e. completely untested!

From: Adrian Hunter <adrian.hunter@intel.com>
Date: Tue, 15 Aug 2017 10:22:09 +0300
Subject: [PATCH] mmc: sdhci: Tidy reading 136-bit responses

Read each register only once and move the code to a separate function so
that it is not jammed against the 80 column margin.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 32a0e8f6fb04..21d0b36d642f 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1173,6 +1173,23 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 }
 EXPORT_SYMBOL_GPL(sdhci_send_command);
 
+static void sdhci_read_rsp_136(struct sdhci_host *host, struct mmc_command *cmd)
+{
+	int i, reg;
+
+	for (i = 0; i < 4; i++) {
+		reg = SDHCI_RESPONSE + (3 - i) * 4;
+		cmd->resp[i] = sdhci_readl(host, reg);
+	}
+
+	/* CRC is stripped so we need to do some shifting */
+	for (i = 0; i < 4; i++) {
+		cmd->resp[i] =<< 8;
+		if (i != 3)
+			cmd->resp[i] |= cmd->resp[i + 1] >> 24;
+	}
+}
+
 static void sdhci_finish_command(struct sdhci_host *host)
 {
 	struct mmc_command *cmd = host->cmd;
@@ -1182,15 +1199,7 @@ static void sdhci_finish_command(struct sdhci_host *host)
 
 	if (cmd->flags & MMC_RSP_PRESENT) {
 		if (cmd->flags & MMC_RSP_136) {
-			/* CRC is stripped so we need to do some shifting. */
-			for (i = 0;i < 4;i++) {
-				cmd->resp[i] = sdhci_readl(host,
-					SDHCI_RESPONSE + (3-i)*4) << 8;
-				if (i != 3)
-					cmd->resp[i] |=
-						sdhci_readb(host,
-						SDHCI_RESPONSE + (3-i)*4-1);
-			}
+			sdhci_read_rsp_136(host, cmd);
 		} else {
 			cmd->resp[0] = sdhci_readl(host, SDHCI_RESPONSE);
 		}
-- 
1.9.1




>  		} else {
>  			cmd->resp[0] = sdhci_readl(host, SDHCI_RESPONSE);
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 0469fa191493..6905131f603d 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -435,6 +435,8 @@ struct sdhci_host {
>  #define SDHCI_QUIRK2_ACMD23_BROKEN			(1<<14)
>  /* Broken Clock divider zero in controller */
>  #define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN		(1<<15)
> +/* Controller does not have CRC stripped in Command Response */
> +#define SDHCI_QUIRK2_NO_CRC_STRIPPING			(1<<16)
>  
>  	int irq;		/* Device IRQ */
>  	void __iomem *ioaddr;	/* Mapped address */
> 

Re: [RFC PATCH 2/7] mmc: sdhci: Add quirk to indicate controller supports ADMA2

[Adrian Hunter]

On 07/08/17 19:01, Kishon Vijay Abraham I wrote:
> TI's implementation of sdhci controller used in DRA7 SoC's doesn't
> have SDHCI_CAN_DO_ADMA2 set in CAPA register though it supports
> ADMA2. Add quirk to support using ADMA2 even if the controller reports
> incorrect capability in CAPA.

A quirk is not needed for this.  Just call sdhci_read_caps() and change
host->caps before calling sdhci_add_host().

> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/mmc/host/sdhci.c | 3 ++-
>  drivers/mmc/host/sdhci.h | 2 ++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index ece3751d2a25..fff0baadbc3e 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -3264,7 +3264,8 @@ int sdhci_setup_host(struct sdhci_host *host)
>  	}
>  
>  	if ((host->version >= SDHCI_SPEC_200) &&
> -		(host->caps & SDHCI_CAN_DO_ADMA2))
> +		((host->caps & SDHCI_CAN_DO_ADMA2) ||
> +		(host->quirks2 & SDHCI_QUIRK2_FORCE_ADMA)))
>  		host->flags |= SDHCI_USE_ADMA;
>  
>  	if ((host->quirks & SDHCI_QUIRK_BROKEN_ADMA) &&
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 6905131f603d..d778034e324d 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -437,6 +437,8 @@ struct sdhci_host {
>  #define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN		(1<<15)
>  /* Controller does not have CRC stripped in Command Response */
>  #define SDHCI_QUIRK2_NO_CRC_STRIPPING			(1<<16)
> +/* Controller has bad caps bits, but really supports DMA */
> +#define SDHCI_QUIRK2_FORCE_ADMA				(1<<17)
>  
>  	int irq;		/* Device IRQ */
>  	void __iomem *ioaddr;	/* Mapped address */
> 

Re: [RFC PATCH 3/7] mmc: sdhci: Add callback to set bus mode

[Adrian Hunter]

On 07/08/17 19:01, Kishon Vijay Abraham I wrote:
> Add callback to set bus mode in sdhci library so that the
> controller driver can perform any bus mode specific configurations
> in the callback function.

A quirk isn't needed.  Just hook ->set_ios() e.g.

	host->mmc_host_ops.set_ios = sdhci_omap_set_ios;

Then sdhci_omap_set_ios() can call sdhci_set_ios() and then do the bus mode
setting.

> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/mmc/host/sdhci.c | 3 +++
>  drivers/mmc/host/sdhci.h | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index fff0baadbc3e..d5050ec42481 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1747,6 +1747,9 @@ void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  	if (host->quirks & SDHCI_QUIRK_RESET_CMD_DATA_ON_IOS)
>  		sdhci_do_reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA);
>  
> +	if (host->ops->set_bus_mode)
> +		host->ops->set_bus_mode(host, ios->bus_mode);
> +
>  	mmiowb();
>  }
>  EXPORT_SYMBOL_GPL(sdhci_set_ios);
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index d778034e324d..39c7a2723906 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -583,6 +583,7 @@ struct sdhci_ops {
>  	void    (*adma_workaround)(struct sdhci_host *host, u32 intmask);
>  	void    (*card_event)(struct sdhci_host *host);
>  	void	(*voltage_switch)(struct sdhci_host *host);
> +	void	(*set_bus_mode)(struct sdhci_host *host, unsigned int mode);
>  };
>  
>  #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
> 

Re: [RFC PATCH 4/7] mmc: sdhci: Add quirk to indicate broken POWER_CONTROL

[Adrian Hunter]

On 07/08/17 19:01, Kishon Vijay Abraham I wrote:
> TI's implementation of sdhci controller used in DRA7 SoC's uses
> POWER_CONTROL register for configuring IO voltage and not
> for core voltage (vdd) which is it's intended use. Add a quirk
> to indicate broken POWER_CONTROL register.

A quirk isn't needed for this.  Use the ->set_power() callback instead.

> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/mmc/host/sdhci.c | 6 ++++++
>  drivers/mmc/host/sdhci.h | 2 ++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index d5050ec42481..a7465a56e6b5 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1419,6 +1419,9 @@ static void sdhci_set_power_reg(struct sdhci_host *host, unsigned char mode,
>  
>  	mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
>  
> +	if (host->quirks2 & SDHCI_QUIRK2_BROKEN_POWER_CONTROL)
> +		return;
> +
>  	if (mode != MMC_POWER_OFF)
>  		sdhci_writeb(host, SDHCI_POWER_ON, SDHCI_POWER_CONTROL);
>  	else
> @@ -1430,6 +1433,9 @@ void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
>  {
>  	u8 pwr = 0;
>  
> +	if (host->quirks2 & SDHCI_QUIRK2_BROKEN_POWER_CONTROL)
> +		return;
> +
>  	if (mode != MMC_POWER_OFF) {
>  		switch (1 << vdd) {
>  		case MMC_VDD_165_195:
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 39c7a2723906..ba94cca7c892 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -439,6 +439,8 @@ struct sdhci_host {
>  #define SDHCI_QUIRK2_NO_CRC_STRIPPING			(1<<16)
>  /* Controller has bad caps bits, but really supports DMA */
>  #define SDHCI_QUIRK2_FORCE_ADMA				(1<<17)
> +/* Controller uses POWER_CONTROL for managing IO voltage and not core voltage */
> +#define SDHCI_QUIRK2_BROKEN_POWER_CONTROL		(1<<18)
>  
>  	int irq;		/* Device IRQ */
>  	void __iomem *ioaddr;	/* Mapped address */
> 

Re: [RFC PATCH 6/7] mmc: sdhci-omap: Add OMAP SDHCI driver

[Adrian Hunter]

On 07/08/17 19:01, Kishon Vijay Abraham I wrote:
> Create a new sdhci-omap driver to configure the eMMC/SD/SDIO controller
> in TI's OMAP SoCs making use of the SDHCI core library. For OMAP specific
> configurations, populate sdhci_ops with OMAP specific callbacks and use
> SDHCI quirks.
> Enable only high speed mode for both SD and eMMC here and add other
> UHS mode support later.

I only have a couple of comments below.

> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/mmc/host/Kconfig      |  12 +
>  drivers/mmc/host/Makefile     |   1 +
>  drivers/mmc/host/sdhci-omap.c | 593 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 606 insertions(+)
>  create mode 100644 drivers/mmc/host/sdhci-omap.c
> 
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 5755b69f2f72..9aa7e5ce43ba 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -871,3 +871,15 @@ config MMC_SDHCI_XENON
>  	  This selects Marvell Xenon eMMC/SD/SDIO SDHCI.
>  	  If you have a controller with this interface, say Y or M here.
>  	  If unsure, say N.
> +
> +config MMC_SDHCI_OMAP
> +	tristate "TI SDHCI Controller Support"
> +	depends on MMC_SDHCI_PLTFM
> +	help
> +	  This selects the Secure Digital Host Controller Interface (SDHCI)
> +	  support present in TI's DRA7 SOCs. The controller supports
> +	  SD/MMC/SDIO devices.
> +
> +	  If you have a controller with this interface, say Y or M here.
> +
> +	  If unsure, say N.
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index 4d4547116311..30fb8b0fd0e1 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -82,6 +82,7 @@ obj-$(CONFIG_MMC_SDHCI_MSM)		+= sdhci-msm.o
>  obj-$(CONFIG_MMC_SDHCI_ST)		+= sdhci-st.o
>  obj-$(CONFIG_MMC_SDHCI_MICROCHIP_PIC32)	+= sdhci-pic32.o
>  obj-$(CONFIG_MMC_SDHCI_BRCMSTB)		+= sdhci-brcmstb.o
> +obj-$(CONFIG_MMC_SDHCI_OMAP)		+= sdhci-omap.o
>  
>  ifeq ($(CONFIG_CB710_DEBUG),y)
>  	CFLAGS-cb710-mmc	+= -DDEBUG
> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
> new file mode 100644
> index 000000000000..c067c428a0cd
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-omap.c
> @@ -0,0 +1,593 @@
> +/*
> + * SDHCI Controller driver for TI's OMAP SoCs
> + *
> + * Copyright (C) 2016-2017 Texas Instruments Incorporated - http://www.ti.com
> + *
> + * Authors: Kishon Vijay Abraham I <kishon@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/mmc/slot-gpio.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include "sdhci-pltfm.h"
> +
> +#define SDHCI_OMAP_CON		0x12c
> +#define CON_DW8			BIT(5)
> +#define CON_DMA_MASTER		BIT(20)
> +#define CON_INIT		BIT(1)
> +#define CON_OD			BIT(0)
> +
> +#define SDHCI_OMAP_CMD		0x20c
> +
> +#define SDHCI_OMAP_HCTL		0x228
> +#define HCTL_SDBP		BIT(8)
> +#define HCTL_SDVS_SHIFT		9
> +#define HCTL_SDVS_MASK		(0x7 << HCTL_SDVS_SHIFT)
> +#define HCTL_SDVS_33		(0x7 << HCTL_SDVS_SHIFT)
> +#define HCTL_SDVS_30		(0x6 << HCTL_SDVS_SHIFT)
> +#define HCTL_SDVS_18		(0x5 << HCTL_SDVS_SHIFT)
> +
> +#define SDHCI_OMAP_SYSCTL	0x22c
> +#define SYSCTL_CEN		BIT(2)
> +#define SYSCTL_CLKD_SHIFT	6
> +#define SYSCTL_CLKD_MASK	0x3ff
> +
> +#define SDHCI_OMAP_STAT		0x230
> +
> +#define SDHCI_OMAP_IE		0x234
> +#define INT_CC_EN		BIT(0)
> +
> +#define SDHCI_OMAP_AC12		0x23c
> +#define AC12_V1V8_SIGEN		BIT(19)
> +
> +#define SDHCI_OMAP_CAPA		0x240
> +#define CAPA_VS33		BIT(24)
> +#define CAPA_VS30		BIT(25)
> +#define CAPA_VS18		BIT(26)
> +
> +#define MMC_TIMEOUT		1		/* 1 msec */

This name "MMC_TIMEOUT" is too generic.  SDHCI_OMAP_TIMEOUT would be better.

> +
> +#define SYSCTL_CLKD_MAX		0x3FF
> +
> +#define IOV_1V8			1800000		/* 180000 uV */
> +#define IOV_3V0			3000000		/* 300000 uV */
> +#define IOV_3V3			3300000		/* 330000 uV */
> +
> +struct sdhci_omap_data {
> +	u32 offset;
> +};
> +
> +struct sdhci_omap_host {
> +	void __iomem		*base;
> +	struct device		*dev;
> +	struct	regulator	*pbias;
> +	bool			pbias_enabled;
> +	struct sdhci_host	*host;
> +	unsigned int		flags;
> +
> +#define SDHCI_OMAP_SUPPORTS_DUAL_VOLT	BIT(0)
> +#define SDHCI_OMAP_NO_1_8_V		BIT(1)
> +
> +	u8			power_mode;
> +	bool			io_3_3v_support;
> +};
> +
> +static inline u32 sdhci_omap_readl(struct sdhci_omap_host *host,
> +				   unsigned int offset)
> +{
> +	return readl(host->base + offset);
> +}
> +
> +static inline void sdhci_omap_writel(struct sdhci_omap_host *host,
> +				     unsigned int offset, u32 data)
> +{
> +	writel(data, host->base + offset);
> +}
> +
> +static int sdhci_omap_set_pbias(struct sdhci_omap_host *omap_host,
> +				bool power_on, unsigned int iov)
> +{
> +	int ret;
> +	struct device *dev = omap_host->dev;
> +
> +	if (IS_ERR(omap_host->pbias))
> +		return 0;
> +
> +	if (power_on) {
> +		ret = regulator_set_voltage(omap_host->pbias, iov, iov);
> +		if (ret) {
> +			dev_err(dev, "pbias set voltage failed\n");
> +			return ret;
> +		}
> +
> +		if (omap_host->pbias_enabled)
> +			return 0;
> +
> +		ret = regulator_enable(omap_host->pbias);
> +		if (ret) {
> +			dev_err(dev, "pbias reg enable fail\n");
> +			return ret;
> +		}
> +
> +		omap_host->pbias_enabled = true;
> +	} else {
> +		if (!omap_host->pbias_enabled)
> +			return 0;
> +
> +		ret = regulator_disable(omap_host->pbias);
> +		if (ret) {
> +			dev_err(dev, "pbias reg disable fail\n");
> +			return ret;
> +		}
> +		omap_host->pbias_enabled = false;
> +	}
> +
> +	return 0;
> +}
> +
> +static int sdhci_omap_enable_iov(struct sdhci_omap_host *omap_host,
> +				 unsigned int iov)
> +{
> +	int ret;
> +	struct sdhci_host *host = omap_host->host;
> +	struct mmc_host *mmc = host->mmc;
> +
> +	ret = sdhci_omap_set_pbias(omap_host, false, 0);
> +	if (ret)
> +		return ret;
> +
> +	if (!IS_ERR(mmc->supply.vqmmc)) {
> +		ret = regulator_set_voltage(mmc->supply.vqmmc, iov, iov);
> +		if (ret) {
> +			dev_err(mmc_dev(mmc), "vqmmc set voltage failed\n");
> +			return ret;
> +		}
> +	}
> +
> +	ret = sdhci_omap_set_pbias(omap_host, true, iov);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static void sdhci_omap_conf_bus_power(struct sdhci_omap_host *omap_host,
> +				      unsigned char signal_voltage)
> +{
> +	u32 reg;
> +	ktime_t timeout;
> +
> +	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_HCTL);
> +	reg &= ~HCTL_SDVS_MASK;
> +
> +	if (signal_voltage == MMC_SIGNAL_VOLTAGE_180)
> +		reg |= HCTL_SDVS_18;
> +	else if (omap_host->io_3_3v_support)
> +		reg |= HCTL_SDVS_33;
> +	else
> +		reg |= HCTL_SDVS_30;
> +
> +	sdhci_omap_writel(omap_host, SDHCI_OMAP_HCTL, reg);
> +
> +	reg |= HCTL_SDBP;
> +	sdhci_omap_writel(omap_host, SDHCI_OMAP_HCTL, reg);
> +
> +	/* wait 1ms */
> +	timeout = ktime_add_ms(ktime_get(), MMC_TIMEOUT);
> +	while (!(sdhci_omap_readl(omap_host, SDHCI_OMAP_HCTL) & HCTL_SDBP)) {
> +		if (WARN_ON(ktime_after(ktime_get(), timeout)))
> +			return;
> +		usleep_range(5, 10);
> +	}
> +}
> +
> +static int sdhci_omap_start_signal_voltage_switch(struct mmc_host *mmc,
> +						  struct mmc_ios *ios)
> +{
> +	u32 reg;
> +	int ret;
> +	unsigned int iov;
> +	struct sdhci_host *host = mmc_priv(mmc);
> +	struct sdhci_pltfm_host *pltfm_host;
> +	struct sdhci_omap_host *omap_host;
> +	struct device *dev;
> +
> +	pltfm_host = sdhci_priv(host);
> +	omap_host = sdhci_pltfm_priv(pltfm_host);
> +	dev = omap_host->dev;
> +
> +	if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
> +		reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CAPA);
> +		if (!(reg & (CAPA_VS33 | CAPA_VS30)))
> +			return -EOPNOTSUPP;
> +
> +		sdhci_omap_conf_bus_power(omap_host, ios->signal_voltage);
> +
> +		reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_AC12);
> +		reg &= ~AC12_V1V8_SIGEN;
> +		sdhci_omap_writel(omap_host, SDHCI_OMAP_AC12, reg);
> +
> +		if (omap_host->io_3_3v_support)
> +			iov = IOV_3V3;
> +		else
> +			iov = IOV_3V0;
> +	} else if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_180) {
> +		reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CAPA);
> +		if (!(reg & CAPA_VS18))
> +			return -EOPNOTSUPP;
> +
> +		sdhci_omap_conf_bus_power(omap_host, ios->signal_voltage);
> +
> +		reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_AC12);
> +		reg |= AC12_V1V8_SIGEN;
> +		sdhci_omap_writel(omap_host, SDHCI_OMAP_AC12, reg);
> +
> +		iov = IOV_1V8;
> +	} else {
> +		return -EOPNOTSUPP;
> +	}
> +
> +	ret = sdhci_omap_enable_iov(omap_host, iov);
> +	if (ret) {
> +		dev_err(dev, "failed to switch IO voltage to %dmV\n", iov);
> +		return ret;
> +	}
> +
> +	dev_dbg(dev, "IO voltage switched to %dmV\n", iov);
> +	return 0;
> +}
> +
> +static void sdhci_omap_set_capabilities(struct sdhci_omap_host *omap_host)
> +{
> +	u32 reg;
> +
> +	/* voltage capabilities might be set by boot loader, clear it */
> +	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CAPA);
> +	reg &= ~(CAPA_VS18 | CAPA_VS30 | CAPA_VS33);
> +
> +	if (omap_host->flags & SDHCI_OMAP_SUPPORTS_DUAL_VOLT) {
> +		if (omap_host->io_3_3v_support)
> +			reg |= (CAPA_VS33 | CAPA_VS18);
> +		else
> +			reg |= (CAPA_VS30 | CAPA_VS18);
> +	} else if (omap_host->flags & SDHCI_OMAP_NO_1_8_V) {
> +		if (omap_host->io_3_3v_support)
> +			reg |= CAPA_VS33;
> +		else
> +			reg |= CAPA_VS30;
> +	} else {
> +		reg |= CAPA_VS18;
> +	}
> +
> +	sdhci_omap_writel(omap_host, SDHCI_OMAP_CAPA, reg);
> +}
> +
> +static void sdhci_omap_set_bus_mode(struct sdhci_host *host, unsigned int mode)
> +{
> +	u32 reg;
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
> +
> +	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CON);
> +	if (mode == MMC_BUSMODE_OPENDRAIN)
> +		reg |= CON_OD;
> +	else
> +		reg &= ~CON_OD;
> +	sdhci_omap_writel(omap_host, SDHCI_OMAP_CON, reg);
> +}
> +
> +static void sdhci_omap_set_bus_width(struct sdhci_host *host, int width)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
> +	u32 reg;
> +
> +	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CON);
> +	if (width == MMC_BUS_WIDTH_8)
> +		reg |= CON_DW8;
> +	else
> +		reg &= ~CON_DW8;
> +	sdhci_omap_writel(omap_host, SDHCI_OMAP_CON, reg);
> +
> +	sdhci_set_bus_width(host, width);
> +}
> +
> +static void sdhci_omap_init_74_clocks(struct sdhci_host *host, u8 power_mode)
> +{
> +	u32 reg;
> +	ktime_t timeout;
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
> +
> +	if (omap_host->power_mode == power_mode)
> +		return;
> +
> +	if (power_mode != MMC_POWER_ON)
> +		return;
> +
> +	disable_irq(host->irq);
> +
> +	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CON);
> +	reg |= CON_INIT;
> +	sdhci_omap_writel(omap_host, SDHCI_OMAP_CON, reg);
> +	sdhci_omap_writel(omap_host, SDHCI_OMAP_CMD, 0x0);
> +
> +	/* wait 1ms */
> +	timeout = ktime_add_ms(ktime_get(), MMC_TIMEOUT);
> +	while (!(sdhci_omap_readl(omap_host, SDHCI_OMAP_STAT) & INT_CC_EN)) {
> +		if (WARN_ON(ktime_after(ktime_get(), timeout)))
> +			return;
> +		usleep_range(5, 10);
> +	}
> +
> +	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CON);
> +	reg &= ~CON_INIT;
> +	sdhci_omap_writel(omap_host, SDHCI_OMAP_CON, reg);
> +	sdhci_omap_writel(omap_host, SDHCI_OMAP_STAT, INT_CC_EN);
> +
> +	enable_irq(host->irq);
> +
> +	omap_host->power_mode = power_mode;
> +}
> +
> +static u16 sdhci_omap_calc_divisor(struct sdhci_pltfm_host *host,
> +				   unsigned int clock)
> +{
> +	u16 dsor;
> +
> +	dsor = DIV_ROUND_UP(clk_get_rate(host->clk), clock);
> +	if (dsor > SYSCTL_CLKD_MAX)
> +		dsor = SYSCTL_CLKD_MAX;
> +
> +	return dsor;
> +}
> +
> +static void sdhci_omap_start_clock(struct sdhci_omap_host *omap_host)
> +{
> +	u32 reg;
> +
> +	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_SYSCTL);
> +	reg |= SYSCTL_CEN;
> +	sdhci_omap_writel(omap_host, SDHCI_OMAP_SYSCTL, reg);
> +}
> +
> +static void sdhci_omap_stop_clock(struct sdhci_omap_host *omap_host)
> +{
> +	u32 reg;
> +
> +	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_SYSCTL);
> +	reg &= ~SYSCTL_CEN;
> +	sdhci_omap_writel(omap_host, SDHCI_OMAP_SYSCTL, reg);
> +}
> +
> +static void sdhci_omap_set_clock(struct sdhci_host *host, unsigned int clock)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
> +	unsigned long clkdiv;
> +
> +	if (!clock)
> +		return;
> +
> +	sdhci_omap_stop_clock(omap_host);
> +
> +	clkdiv = sdhci_omap_calc_divisor(pltfm_host, clock);
> +	clkdiv = (clkdiv & SYSCTL_CLKD_MASK) << SYSCTL_CLKD_SHIFT;
> +	sdhci_enable_clk(host, clkdiv);
> +
> +	sdhci_omap_start_clock(omap_host);
> +}
> +
> +static int sdhci_omap_enable_dma(struct sdhci_host *host)
> +{
> +	u32 reg;
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
> +
> +	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CON);
> +	reg |= CON_DMA_MASTER;
> +	sdhci_omap_writel(omap_host, SDHCI_OMAP_CON, reg);
> +
> +	return 0;
> +}
> +
> +unsigned int sdhci_omap_get_min_clock(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +
> +	return clk_get_rate(pltfm_host->clk) / SYSCTL_CLKD_MAX;
> +}
> +
> +static struct sdhci_ops sdhci_omap_ops = {
> +	.set_clock = sdhci_omap_set_clock,
> +	.enable_dma = sdhci_omap_enable_dma,
> +	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
> +	.get_min_clock = sdhci_omap_get_min_clock,
> +	.set_bus_width = sdhci_omap_set_bus_width,
> +	.platform_send_init_74_clocks = sdhci_omap_init_74_clocks,
> +	.reset = sdhci_reset,
> +	.set_uhs_signaling = sdhci_set_uhs_signaling,
> +	.set_bus_mode = sdhci_omap_set_bus_mode,
> +};
> +
> +void sdhci_omap_get_of_property(struct sdhci_omap_host *omap_host)
> +{
> +	struct device *dev = omap_host->dev;
> +	struct sdhci_host *host = omap_host->host;
> +	struct mmc_host *mmc = host->mmc;
> +
> +	if (device_property_read_bool(dev, "ti,dual-volt"))
> +		omap_host->flags |= SDHCI_OMAP_SUPPORTS_DUAL_VOLT;
> +
> +	if (device_property_read_bool(dev, "no-1-8-v"))
> +		omap_host->flags |= SDHCI_OMAP_NO_1_8_V;
> +
> +	if (device_property_read_bool(dev, "ti,non-removable"))
> +		mmc->caps |= MMC_CAP_NONREMOVABLE;
> +}
> +
> +static const struct sdhci_pltfm_data sdhci_omap_pdata = {
> +	.quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION |
> +		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
> +		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> +		  SDHCI_QUIRK_NO_HISPD_BIT |
> +		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
> +	.quirks2 = SDHCI_QUIRK2_NO_1_8_V |
> +		   SDHCI_QUIRK2_ACMD23_BROKEN |
> +		   SDHCI_QUIRK2_NO_CRC_STRIPPING |
> +		   SDHCI_QUIRK2_FORCE_ADMA |
> +		   SDHCI_QUIRK2_BROKEN_POWER_CONTROL,
> +	.ops = &sdhci_omap_ops,
> +};
> +
> +static const struct sdhci_omap_data dra7_data = {
> +	.offset = 0x200,
> +};
> +
> +static const struct of_device_id omap_sdhci_match[] = {
> +	{ .compatible = "ti,dra7-sdhci", .data = &dra7_data },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, omap_sdhci_match);
> +
> +static int sdhci_omap_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	u32 offset;
> +	struct device *dev = &pdev->dev;
> +	struct sdhci_host *host;
> +	struct sdhci_pltfm_host *pltfm_host;
> +	struct sdhci_omap_host *omap_host;
> +	struct mmc_host *mmc;
> +	const struct of_device_id *match;
> +	struct sdhci_omap_data *data;
> +
> +	match = of_match_device(omap_sdhci_match, dev);
> +	if (!match)
> +		return -EINVAL;
> +
> +	data = (struct sdhci_omap_data *)match->data;
> +	if (!data) {
> +		dev_err(dev, "no sdhci omap data\n");
> +		return -EINVAL;
> +	}
> +
> +	offset = data->offset;
> +
> +	host = sdhci_pltfm_init(pdev, &sdhci_omap_pdata,
> +				sizeof(*omap_host));
> +	if (IS_ERR(host)) {
> +		dev_err(dev, "Failed sdhci_pltfm_init\n");
> +		return PTR_ERR(host);
> +	}
> +
> +	pltfm_host = sdhci_priv(host);
> +	omap_host = sdhci_pltfm_priv(pltfm_host);
> +	omap_host->host = host;
> +	omap_host->base = host->ioaddr;
> +	omap_host->dev = dev;
> +	sdhci_omap_get_of_property(omap_host);
> +
> +	host->ioaddr += offset;
> +
> +	mmc = host->mmc;
> +	ret = mmc_of_parse(mmc);
> +	if (ret)
> +		goto err_of_parse;
> +
> +	pltfm_host->clk = devm_clk_get(dev, "fck");
> +	if (IS_ERR(pltfm_host->clk)) {
> +		ret = PTR_ERR(pltfm_host->clk);
> +		goto err_of_parse;
> +	}
> +
> +	ret = clk_set_rate(pltfm_host->clk, mmc->f_max);
> +	if (ret) {
> +		dev_err(dev, "failed to set clock to %d\n", mmc->f_max);
> +		goto err_of_parse;
> +	}
> +
> +	omap_host->pbias = devm_regulator_get_optional(dev, "pbias");
> +	if (IS_ERR(omap_host->pbias)) {
> +		ret = PTR_ERR(omap_host->pbias);
> +		if (ret != -ENODEV) {
> +			dev_err(dev, "CONFIG_REGULATOR_PBIAS not enabled??\n");
> +			return ret;
> +		}
> +		dev_dbg(dev, "unable to get pbias regulator %ld\n",
> +			PTR_ERR(omap_host->pbias));
> +	} else {
> +		if (regulator_is_supported_voltage(omap_host->pbias, IOV_3V3,
> +						   IOV_3V3))
> +			omap_host->io_3_3v_support = true;
> +		dev_vdbg(dev, "Max PBIAS support Voltage: %dmv\n",
> +			 omap_host->io_3_3v_support ? IOV_3V3 : IOV_3V0);
> +	}
> +	omap_host->pbias_enabled = false;
> +
> +	pm_runtime_enable(dev);
> +	ret = pm_runtime_get_sync(dev);

What are you trying to do with runtime pm?  There don't seem to be any pm
callbacks, so does this do anything?

> +	if (ret < 0) {
> +		dev_err(dev, "pm_runtime_get_sync failed\n");
> +		goto err_get_sync;
> +	}
> +
> +	sdhci_omap_set_capabilities(omap_host);
> +
> +	host->mmc_host_ops.get_ro = mmc_gpio_get_ro;
> +	host->mmc_host_ops.start_signal_voltage_switch =
> +					sdhci_omap_start_signal_voltage_switch;
> +
> +	ret = sdhci_add_host(host);
> +	if (ret)
> +		goto err_get_sync;
> +
> +	return 0;
> +
> +err_get_sync:
> +	pm_runtime_put_sync(dev);
> +	pm_runtime_disable(dev);
> +
> +err_of_parse:
> +	sdhci_pltfm_free(pdev);
> +	return ret;
> +}
> +
> +static int sdhci_omap_remove(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +
> +	sdhci_remove_host(host, true);
> +	pm_runtime_put_sync(dev);
> +	pm_runtime_disable(dev);
> +	sdhci_pltfm_free(pdev);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver sdhci_omap_driver = {
> +	.probe = sdhci_omap_probe,
> +	.remove = sdhci_omap_remove,
> +	.driver = {
> +		   .name = "sdhci-omap",
> +		   .of_match_table = of_match_ptr(omap_sdhci_match),
> +		  },
> +};
> +
> +module_platform_driver(sdhci_omap_driver);
> +
> +MODULE_DESCRIPTION("SDHCI driver for OMAP SoCs");
> +MODULE_AUTHOR("Texas Instruments Inc.");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("OMAP SDHCI Driver");
> 

Re: [RFC PATCH 1/7] mmc: sdhci: Add quirk to indicate no CRC stripping in MMC_RSP_136

[Kishon Vijay Abraham I]

Hi Adrian,

On Tuesday 15 August 2017 12:57 PM, Adrian Hunter wrote:
> On 07/08/17 19:01, Kishon Vijay Abraham I wrote:
>> TI's implementation of sdhci controller used in DRA7 SoC's doesn't
>> strip CRC in responses with length 136 bits. Add quirk to indicate
>> the controller does not strip CRC in MMC_RSP_136. If this quirk is
>> set sdhci library shouldn't shift the response present in
>> SDHCI_RESPONSE register.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>  drivers/mmc/host/sdhci.c | 27 +++++++++++++++++++--------
>>  drivers/mmc/host/sdhci.h |  2 ++
>>  2 files changed, 21 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index ecd0d4350e8a..ece3751d2a25 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1182,14 +1182,25 @@ static void sdhci_finish_command(struct sdhci_host *host)
>>  
>>  	if (cmd->flags & MMC_RSP_PRESENT) {
>>  		if (cmd->flags & MMC_RSP_136) {
>> -			/* CRC is stripped so we need to do some shifting. */
>> -			for (i = 0;i < 4;i++) {
>> -				cmd->resp[i] = sdhci_readl(host,
>> -					SDHCI_RESPONSE + (3-i)*4) << 8;
>> -				if (i != 3)
>> -					cmd->resp[i] |=
>> -						sdhci_readb(host,
>> -						SDHCI_RESPONSE + (3-i)*4-1);
>> +			if (!(host->quirks2 & SDHCI_QUIRK2_NO_CRC_STRIPPING)) {
> 
> This is about the 136-bit response so let's put that in the quirk name.  How about SDHCI_QUIRK2_RSP_136_HAS_CRC

Since omap seems to be the only platform that doesn't have CRC, I prefer adding
SDHCI_QUIRK2_RSP_136_NO_CRC. That way we can add the quirk only in sdhci-omap
instead of all the existing sdhci drivers.

> 
>> +				/*
>> +				 * CRC is stripped so we need to do some
>> +				 * shifting.
>> +				 */
>> +				for (i = 0; i < 4; i++) {
>> +					cmd->resp[i] =
>> +						sdhci_readl(host, SDHCI_RESPONSE
>> +							    + (3 - i) * 4) << 8;
>> +					if (i != 3)
>> +						cmd->resp[i] |=
>> +						sdhci_readb(host, SDHCI_RESPONSE
>> +							    + (3 - i) * 4 - 1);
>> +				}
>> +			} else {
>> +				for (i = 0; i < 4; i++)
>> +					cmd->resp[i] =
>> +					sdhci_readl(host, SDHCI_RESPONSE +
>> +						    (3 - i) * 4);
>>  			}
> 
> This is all very jammed up against the 80 column margin.  Please make a new patch to separate it into a new function sdhci_read_rsp_136() and then another patch to add the quirk.
> i.e. completely untested!

Sure. Thanks for the patch.

Thanks
Kishon

Re: [RFC PATCH 2/7] mmc: sdhci: Add quirk to indicate controller supports ADMA2

[Kishon Vijay Abraham I]

On Tuesday 15 August 2017 01:03 PM, Adrian Hunter wrote:
> On 07/08/17 19:01, Kishon Vijay Abraham I wrote:
>> TI's implementation of sdhci controller used in DRA7 SoC's doesn't
>> have SDHCI_CAN_DO_ADMA2 set in CAPA register though it supports
>> ADMA2. Add quirk to support using ADMA2 even if the controller reports
>> incorrect capability in CAPA.
> 
> A quirk is not needed for this.  Just call sdhci_read_caps() and change
> host->caps before calling sdhci_add_host().

Okay, got it.

Thanks
Kishon
> 
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>  drivers/mmc/host/sdhci.c | 3 ++-
>>  drivers/mmc/host/sdhci.h | 2 ++
>>  2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index ece3751d2a25..fff0baadbc3e 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -3264,7 +3264,8 @@ int sdhci_setup_host(struct sdhci_host *host)
>>  	}
>>  
>>  	if ((host->version >= SDHCI_SPEC_200) &&
>> -		(host->caps & SDHCI_CAN_DO_ADMA2))
>> +		((host->caps & SDHCI_CAN_DO_ADMA2) ||
>> +		(host->quirks2 & SDHCI_QUIRK2_FORCE_ADMA)))
>>  		host->flags |= SDHCI_USE_ADMA;
>>  
>>  	if ((host->quirks & SDHCI_QUIRK_BROKEN_ADMA) &&
>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>> index 6905131f603d..d778034e324d 100644
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -437,6 +437,8 @@ struct sdhci_host {
>>  #define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN		(1<<15)
>>  /* Controller does not have CRC stripped in Command Response */
>>  #define SDHCI_QUIRK2_NO_CRC_STRIPPING			(1<<16)
>> +/* Controller has bad caps bits, but really supports DMA */
>> +#define SDHCI_QUIRK2_FORCE_ADMA				(1<<17)
>>  
>>  	int irq;		/* Device IRQ */
>>  	void __iomem *ioaddr;	/* Mapped address */
>>
> 

Re: [RFC PATCH 3/7] mmc: sdhci: Add callback to set bus mode

[Kishon Vijay Abraham I]

Adrian,

On Tuesday 15 August 2017 01:08 PM, Adrian Hunter wrote:
> On 07/08/17 19:01, Kishon Vijay Abraham I wrote:
>> Add callback to set bus mode in sdhci library so that the
>> controller driver can perform any bus mode specific configurations
>> in the callback function.
> 
> A quirk isn't needed.  Just hook ->set_ios() e.g.
> 
> 	host->mmc_host_ops.set_ios = sdhci_omap_set_ios;
> 
> Then sdhci_omap_set_ios() can call sdhci_set_ios() and then do the bus mode
> setting.

I'm not adding a quirk here.. but I got your point.

Thanks
Kishon

Re: [RFC PATCH 4/7] mmc: sdhci: Add quirk to indicate broken POWER_CONTROL

[Kishon Vijay Abraham I]

On Tuesday 15 August 2017 01:11 PM, Adrian Hunter wrote:
> On 07/08/17 19:01, Kishon Vijay Abraham I wrote:
>> TI's implementation of sdhci controller used in DRA7 SoC's uses
>> POWER_CONTROL register for configuring IO voltage and not
>> for core voltage (vdd) which is it's intended use. Add a quirk
>> to indicate broken POWER_CONTROL register.
> 
> A quirk isn't needed for this.  Use the ->set_power() callback instead.

sure!

Thanks
Kishon
> 
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>  drivers/mmc/host/sdhci.c | 6 ++++++
>>  drivers/mmc/host/sdhci.h | 2 ++
>>  2 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index d5050ec42481..a7465a56e6b5 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1419,6 +1419,9 @@ static void sdhci_set_power_reg(struct sdhci_host *host, unsigned char mode,
>>  
>>  	mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
>>  
>> +	if (host->quirks2 & SDHCI_QUIRK2_BROKEN_POWER_CONTROL)
>> +		return;
>> +
>>  	if (mode != MMC_POWER_OFF)
>>  		sdhci_writeb(host, SDHCI_POWER_ON, SDHCI_POWER_CONTROL);
>>  	else
>> @@ -1430,6 +1433,9 @@ void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
>>  {
>>  	u8 pwr = 0;
>>  
>> +	if (host->quirks2 & SDHCI_QUIRK2_BROKEN_POWER_CONTROL)
>> +		return;
>> +
>>  	if (mode != MMC_POWER_OFF) {
>>  		switch (1 << vdd) {
>>  		case MMC_VDD_165_195:
>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>> index 39c7a2723906..ba94cca7c892 100644
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -439,6 +439,8 @@ struct sdhci_host {
>>  #define SDHCI_QUIRK2_NO_CRC_STRIPPING			(1<<16)
>>  /* Controller has bad caps bits, but really supports DMA */
>>  #define SDHCI_QUIRK2_FORCE_ADMA				(1<<17)
>> +/* Controller uses POWER_CONTROL for managing IO voltage and not core voltage */
>> +#define SDHCI_QUIRK2_BROKEN_POWER_CONTROL		(1<<18)
>>  
>>  	int irq;		/* Device IRQ */
>>  	void __iomem *ioaddr;	/* Mapped address */
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [RFC PATCH 5/7] dt-bindings: ti-omap-hsmmc: Document new compatible for sdhci omap

[Kishon Vijay Abraham I]

Hi Tony,

On Thursday 10 August 2017 03:42 AM, Tony Lindgren wrote:
> * Kishon Vijay Abraham I <kishon@ti.com> [170807 09:03]:
>> Document the new compatible string "ti,dra7-sdhci" to be used for
>> MMC controllers in DRA7 and DRA72 SoCs.
> 
> I wonder if this should really be documented for sdhci
> instead of ti-omap-hsmmc.txt?

hmm.. yeah, having a separate binding document for sdhci would make the binding
really clean and we also don't have to carry legacy binding code. But we'll be
never able to remove omap-hsmmc driver then and end up maintaining 2 drivers
for the same controller.
> 
> I agree it's better to use sdhci compatible and keep the
> hsmmc compatible around as the sdhci is still missing features
> for runtime PM and context save and restore etc.

yeah, once we have all the features in omap_hsmmc added in sdhci-omap, we can
add the omap-hsmmc compatible to sdhci-omap and remove omap-hsmmc driver.

Thanks
Kishon

Re: [RFC PATCH 6/7] mmc: sdhci-omap: Add OMAP SDHCI driver

[Kishon Vijay Abraham I]

Hi Adrian,

On Tuesday 15 August 2017 01:52 PM, Adrian Hunter wrote:
> On 07/08/17 19:01, Kishon Vijay Abraham I wrote:
>> Create a new sdhci-omap driver to configure the eMMC/SD/SDIO controller
>> in TI's OMAP SoCs making use of the SDHCI core library. For OMAP specific
>> configurations, populate sdhci_ops with OMAP specific callbacks and use
>> SDHCI quirks.
>> Enable only high speed mode for both SD and eMMC here and add other
>> UHS mode support later.
> 
> I only have a couple of comments below.
> 
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>  drivers/mmc/host/Kconfig      |  12 +
>>  drivers/mmc/host/Makefile     |   1 +
>>  drivers/mmc/host/sdhci-omap.c | 593 ++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 606 insertions(+)
>>  create mode 100644 drivers/mmc/host/sdhci-omap.c
>>
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index 5755b69f2f72..9aa7e5ce43ba 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -871,3 +871,15 @@ config MMC_SDHCI_XENON
>>  	  This selects Marvell Xenon eMMC/SD/SDIO SDHCI.
>>  	  If you have a controller with this interface, say Y or M here.
>>  	  If unsure, say N.
>> +
>> +config MMC_SDHCI_OMAP
>> +	tristate "TI SDHCI Controller Support"
>> +	depends on MMC_SDHCI_PLTFM
>> +	help
>> +	  This selects the Secure Digital Host Controller Interface (SDHCI)
>> +	  support present in TI's DRA7 SOCs. The controller supports
>> +	  SD/MMC/SDIO devices.
>> +
>> +	  If you have a controller with this interface, say Y or M here.
>> +
>> +	  If unsure, say N.
>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>> index 4d4547116311..30fb8b0fd0e1 100644
>> --- a/drivers/mmc/host/Makefile
>> +++ b/drivers/mmc/host/Makefile
>> @@ -82,6 +82,7 @@ obj-$(CONFIG_MMC_SDHCI_MSM)		+= sdhci-msm.o
>>  obj-$(CONFIG_MMC_SDHCI_ST)		+= sdhci-st.o
>>  obj-$(CONFIG_MMC_SDHCI_MICROCHIP_PIC32)	+= sdhci-pic32.o
>>  obj-$(CONFIG_MMC_SDHCI_BRCMSTB)		+= sdhci-brcmstb.o
>> +obj-$(CONFIG_MMC_SDHCI_OMAP)		+= sdhci-omap.o
>>  
>>  ifeq ($(CONFIG_CB710_DEBUG),y)
>>  	CFLAGS-cb710-mmc	+= -DDEBUG
>> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
>> new file mode 100644
>> index 000000000000..c067c428a0cd
>> --- /dev/null
>> +++ b/drivers/mmc/host/sdhci-omap.c
>> @@ -0,0 +1,593 @@
>> +/*
>> + * SDHCI Controller driver for TI's OMAP SoCs
>> + *
>> + * Copyright (C) 2016-2017 Texas Instruments Incorporated - http://www.ti.com
>> + *
>> + * Authors: Kishon Vijay Abraham I <kishon@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/mmc/slot-gpio.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/regulator/consumer.h>
>> +
>> +#include "sdhci-pltfm.h"
>> +
>> +#define SDHCI_OMAP_CON		0x12c
>> +#define CON_DW8			BIT(5)
>> +#define CON_DMA_MASTER		BIT(20)
>> +#define CON_INIT		BIT(1)
>> +#define CON_OD			BIT(0)
>> +
>> +#define SDHCI_OMAP_CMD		0x20c
>> +
>> +#define SDHCI_OMAP_HCTL		0x228
>> +#define HCTL_SDBP		BIT(8)
>> +#define HCTL_SDVS_SHIFT		9
>> +#define HCTL_SDVS_MASK		(0x7 << HCTL_SDVS_SHIFT)
>> +#define HCTL_SDVS_33		(0x7 << HCTL_SDVS_SHIFT)
>> +#define HCTL_SDVS_30		(0x6 << HCTL_SDVS_SHIFT)
>> +#define HCTL_SDVS_18		(0x5 << HCTL_SDVS_SHIFT)
>> +
>> +#define SDHCI_OMAP_SYSCTL	0x22c
>> +#define SYSCTL_CEN		BIT(2)
>> +#define SYSCTL_CLKD_SHIFT	6
>> +#define SYSCTL_CLKD_MASK	0x3ff
>> +
>> +#define SDHCI_OMAP_STAT		0x230
>> +
>> +#define SDHCI_OMAP_IE		0x234
>> +#define INT_CC_EN		BIT(0)
>> +
>> +#define SDHCI_OMAP_AC12		0x23c
>> +#define AC12_V1V8_SIGEN		BIT(19)
>> +
>> +#define SDHCI_OMAP_CAPA		0x240
>> +#define CAPA_VS33		BIT(24)
>> +#define CAPA_VS30		BIT(25)
>> +#define CAPA_VS18		BIT(26)
>> +
>> +#define MMC_TIMEOUT		1		/* 1 msec */
> 
> This name "MMC_TIMEOUT" is too generic.  SDHCI_OMAP_TIMEOUT would be better.

okay.
> 
>> +
>> +#define SYSCTL_CLKD_MAX		0x3FF
>> +
>> +#define IOV_1V8			1800000		/* 180000 uV */
>> +#define IOV_3V0			3000000		/* 300000 uV */
>> +#define IOV_3V3			3300000		/* 330000 uV */
>> +
>> +struct sdhci_omap_data {
>> +	u32 offset;
>> +};
>> +
>> +struct sdhci_omap_host {
>> +	void __iomem		*base;
>> +	struct device		*dev;
>> +	struct	regulator	*pbias;
>> +	bool			pbias_enabled;
>> +	struct sdhci_host	*host;
>> +	unsigned int		flags;
>> +
>> +#define SDHCI_OMAP_SUPPORTS_DUAL_VOLT	BIT(0)
>> +#define SDHCI_OMAP_NO_1_8_V		BIT(1)
>> +
>> +	u8			power_mode;
>> +	bool			io_3_3v_support;
>> +};
>> +
.
.
<<snip>>
.
.
>> +static int sdhci_omap_probe(struct platform_device *pdev)
>> +{
>> +	int ret;
>> +	u32 offset;
>> +	struct device *dev = &pdev->dev;
>> +	struct sdhci_host *host;
>> +	struct sdhci_pltfm_host *pltfm_host;
>> +	struct sdhci_omap_host *omap_host;
>> +	struct mmc_host *mmc;
>> +	const struct of_device_id *match;
>> +	struct sdhci_omap_data *data;
>> +
>> +	match = of_match_device(omap_sdhci_match, dev);
>> +	if (!match)
>> +		return -EINVAL;
>> +
>> +	data = (struct sdhci_omap_data *)match->data;
>> +	if (!data) {
>> +		dev_err(dev, "no sdhci omap data\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	offset = data->offset;
>> +
>> +	host = sdhci_pltfm_init(pdev, &sdhci_omap_pdata,
>> +				sizeof(*omap_host));
>> +	if (IS_ERR(host)) {
>> +		dev_err(dev, "Failed sdhci_pltfm_init\n");
>> +		return PTR_ERR(host);
>> +	}
>> +
>> +	pltfm_host = sdhci_priv(host);
>> +	omap_host = sdhci_pltfm_priv(pltfm_host);
>> +	omap_host->host = host;
>> +	omap_host->base = host->ioaddr;
>> +	omap_host->dev = dev;
>> +	sdhci_omap_get_of_property(omap_host);
>> +
>> +	host->ioaddr += offset;
>> +
>> +	mmc = host->mmc;
>> +	ret = mmc_of_parse(mmc);
>> +	if (ret)
>> +		goto err_of_parse;
>> +
>> +	pltfm_host->clk = devm_clk_get(dev, "fck");
>> +	if (IS_ERR(pltfm_host->clk)) {
>> +		ret = PTR_ERR(pltfm_host->clk);
>> +		goto err_of_parse;
>> +	}
>> +
>> +	ret = clk_set_rate(pltfm_host->clk, mmc->f_max);
>> +	if (ret) {
>> +		dev_err(dev, "failed to set clock to %d\n", mmc->f_max);
>> +		goto err_of_parse;
>> +	}
>> +
>> +	omap_host->pbias = devm_regulator_get_optional(dev, "pbias");
>> +	if (IS_ERR(omap_host->pbias)) {
>> +		ret = PTR_ERR(omap_host->pbias);
>> +		if (ret != -ENODEV) {
>> +			dev_err(dev, "CONFIG_REGULATOR_PBIAS not enabled??\n");
>> +			return ret;
>> +		}
>> +		dev_dbg(dev, "unable to get pbias regulator %ld\n",
>> +			PTR_ERR(omap_host->pbias));
>> +	} else {
>> +		if (regulator_is_supported_voltage(omap_host->pbias, IOV_3V3,
>> +						   IOV_3V3))
>> +			omap_host->io_3_3v_support = true;
>> +		dev_vdbg(dev, "Max PBIAS support Voltage: %dmv\n",
>> +			 omap_host->io_3_3v_support ? IOV_3V3 : IOV_3V0);
>> +	}
>> +	omap_host->pbias_enabled = false;
>> +
>> +	pm_runtime_enable(dev);
>> +	ret = pm_runtime_get_sync(dev);
> 
> What are you trying to do with runtime pm?  There don't seem to be any pm
> callbacks, so does this do anything?

yeah, pm_runtime_get_sync enables the functional clock and also configures the
SYSCONFIG regiters present in the controller. It gets the details for
configuring those from arch/arm/mach-omap2/omap_hwmod_*

Thanks
Kishon

Re: [RFC PATCH 1/7] mmc: sdhci: Add quirk to indicate no CRC stripping in MMC_RSP_136

[Adrian Hunter]

On 17/08/17 08:20, Kishon Vijay Abraham I wrote:
> Hi Adrian,
> 
> On Tuesday 15 August 2017 12:57 PM, Adrian Hunter wrote:
>> On 07/08/17 19:01, Kishon Vijay Abraham I wrote:
>>> TI's implementation of sdhci controller used in DRA7 SoC's doesn't
>>> strip CRC in responses with length 136 bits. Add quirk to indicate
>>> the controller does not strip CRC in MMC_RSP_136. If this quirk is
>>> set sdhci library shouldn't shift the response present in
>>> SDHCI_RESPONSE register.
>>>
>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>> ---
>>>  drivers/mmc/host/sdhci.c | 27 +++++++++++++++++++--------
>>>  drivers/mmc/host/sdhci.h |  2 ++
>>>  2 files changed, 21 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index ecd0d4350e8a..ece3751d2a25 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -1182,14 +1182,25 @@ static void sdhci_finish_command(struct sdhci_host *host)
>>>  
>>>  	if (cmd->flags & MMC_RSP_PRESENT) {
>>>  		if (cmd->flags & MMC_RSP_136) {
>>> -			/* CRC is stripped so we need to do some shifting. */
>>> -			for (i = 0;i < 4;i++) {
>>> -				cmd->resp[i] = sdhci_readl(host,
>>> -					SDHCI_RESPONSE + (3-i)*4) << 8;
>>> -				if (i != 3)
>>> -					cmd->resp[i] |=
>>> -						sdhci_readb(host,
>>> -						SDHCI_RESPONSE + (3-i)*4-1);
>>> +			if (!(host->quirks2 & SDHCI_QUIRK2_NO_CRC_STRIPPING)) {
>>
>> This is about the 136-bit response so let's put that in the quirk name.  How about SDHCI_QUIRK2_RSP_136_HAS_CRC
> 
> Since omap seems to be the only platform that doesn't have CRC, I prefer adding
> SDHCI_QUIRK2_RSP_136_NO_CRC. That way we can add the quirk only in sdhci-omap
> instead of all the existing sdhci drivers.

Except that it is the way I described.  All the others strip the CRC and we
have to shift to pretend it is still there.

> 
>>
>>> +				/*
>>> +				 * CRC is stripped so we need to do some
>>> +				 * shifting.
>>> +				 */
>>> +				for (i = 0; i < 4; i++) {
>>> +					cmd->resp[i] =
>>> +						sdhci_readl(host, SDHCI_RESPONSE
>>> +							    + (3 - i) * 4) << 8;
>>> +					if (i != 3)
>>> +						cmd->resp[i] |=
>>> +						sdhci_readb(host, SDHCI_RESPONSE
>>> +							    + (3 - i) * 4 - 1);
>>> +				}
>>> +			} else {
>>> +				for (i = 0; i < 4; i++)
>>> +					cmd->resp[i] =
>>> +					sdhci_readl(host, SDHCI_RESPONSE +
>>> +						    (3 - i) * 4);
>>>  			}
>>
>> This is all very jammed up against the 80 column margin.  Please make a new patch to separate it into a new function sdhci_read_rsp_136() and then another patch to add the quirk.
>> i.e. completely untested!
> 
> Sure. Thanks for the patch.
> 
> Thanks
> Kishon
> 

Re: [RFC PATCH 6/7] mmc: sdhci-omap: Add OMAP SDHCI driver

[Adrian Hunter]

On 17/08/17 08:57, Kishon Vijay Abraham I wrote:
> Hi Adrian,
> 
> On Tuesday 15 August 2017 01:52 PM, Adrian Hunter wrote:
>> On 07/08/17 19:01, Kishon Vijay Abraham I wrote:
>>> Create a new sdhci-omap driver to configure the eMMC/SD/SDIO controller
>>> in TI's OMAP SoCs making use of the SDHCI core library. For OMAP specific
>>> configurations, populate sdhci_ops with OMAP specific callbacks and use
>>> SDHCI quirks.
>>> Enable only high speed mode for both SD and eMMC here and add other
>>> UHS mode support later.
>>
>> I only have a couple of comments below.
>>
>>>
>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>> ---
>>>  drivers/mmc/host/Kconfig      |  12 +
>>>  drivers/mmc/host/Makefile     |   1 +
>>>  drivers/mmc/host/sdhci-omap.c | 593 ++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 606 insertions(+)
>>>  create mode 100644 drivers/mmc/host/sdhci-omap.c
>>>
>>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>>> index 5755b69f2f72..9aa7e5ce43ba 100644
>>> --- a/drivers/mmc/host/Kconfig
>>> +++ b/drivers/mmc/host/Kconfig
>>> @@ -871,3 +871,15 @@ config MMC_SDHCI_XENON
>>>  	  This selects Marvell Xenon eMMC/SD/SDIO SDHCI.
>>>  	  If you have a controller with this interface, say Y or M here.
>>>  	  If unsure, say N.
>>> +
>>> +config MMC_SDHCI_OMAP
>>> +	tristate "TI SDHCI Controller Support"
>>> +	depends on MMC_SDHCI_PLTFM
>>> +	help
>>> +	  This selects the Secure Digital Host Controller Interface (SDHCI)
>>> +	  support present in TI's DRA7 SOCs. The controller supports
>>> +	  SD/MMC/SDIO devices.
>>> +
>>> +	  If you have a controller with this interface, say Y or M here.
>>> +
>>> +	  If unsure, say N.
>>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>>> index 4d4547116311..30fb8b0fd0e1 100644
>>> --- a/drivers/mmc/host/Makefile
>>> +++ b/drivers/mmc/host/Makefile
>>> @@ -82,6 +82,7 @@ obj-$(CONFIG_MMC_SDHCI_MSM)		+= sdhci-msm.o
>>>  obj-$(CONFIG_MMC_SDHCI_ST)		+= sdhci-st.o
>>>  obj-$(CONFIG_MMC_SDHCI_MICROCHIP_PIC32)	+= sdhci-pic32.o
>>>  obj-$(CONFIG_MMC_SDHCI_BRCMSTB)		+= sdhci-brcmstb.o
>>> +obj-$(CONFIG_MMC_SDHCI_OMAP)		+= sdhci-omap.o
>>>  
>>>  ifeq ($(CONFIG_CB710_DEBUG),y)
>>>  	CFLAGS-cb710-mmc	+= -DDEBUG
>>> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
>>> new file mode 100644
>>> index 000000000000..c067c428a0cd
>>> --- /dev/null
>>> +++ b/drivers/mmc/host/sdhci-omap.c
>>> @@ -0,0 +1,593 @@
>>> +/*
>>> + * SDHCI Controller driver for TI's OMAP SoCs
>>> + *
>>> + * Copyright (C) 2016-2017 Texas Instruments Incorporated - http://www.ti.com
>>> + *
>>> + * Authors: Kishon Vijay Abraham I <kishon@ti.com>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#include <linux/delay.h>
>>> +#include <linux/mmc/slot-gpio.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/pm_runtime.h>
>>> +#include <linux/regulator/consumer.h>
>>> +
>>> +#include "sdhci-pltfm.h"
>>> +
>>> +#define SDHCI_OMAP_CON		0x12c
>>> +#define CON_DW8			BIT(5)
>>> +#define CON_DMA_MASTER		BIT(20)
>>> +#define CON_INIT		BIT(1)
>>> +#define CON_OD			BIT(0)
>>> +
>>> +#define SDHCI_OMAP_CMD		0x20c
>>> +
>>> +#define SDHCI_OMAP_HCTL		0x228
>>> +#define HCTL_SDBP		BIT(8)
>>> +#define HCTL_SDVS_SHIFT		9
>>> +#define HCTL_SDVS_MASK		(0x7 << HCTL_SDVS_SHIFT)
>>> +#define HCTL_SDVS_33		(0x7 << HCTL_SDVS_SHIFT)
>>> +#define HCTL_SDVS_30		(0x6 << HCTL_SDVS_SHIFT)
>>> +#define HCTL_SDVS_18		(0x5 << HCTL_SDVS_SHIFT)
>>> +
>>> +#define SDHCI_OMAP_SYSCTL	0x22c
>>> +#define SYSCTL_CEN		BIT(2)
>>> +#define SYSCTL_CLKD_SHIFT	6
>>> +#define SYSCTL_CLKD_MASK	0x3ff
>>> +
>>> +#define SDHCI_OMAP_STAT		0x230
>>> +
>>> +#define SDHCI_OMAP_IE		0x234
>>> +#define INT_CC_EN		BIT(0)
>>> +
>>> +#define SDHCI_OMAP_AC12		0x23c
>>> +#define AC12_V1V8_SIGEN		BIT(19)
>>> +
>>> +#define SDHCI_OMAP_CAPA		0x240
>>> +#define CAPA_VS33		BIT(24)
>>> +#define CAPA_VS30		BIT(25)
>>> +#define CAPA_VS18		BIT(26)
>>> +
>>> +#define MMC_TIMEOUT		1		/* 1 msec */
>>
>> This name "MMC_TIMEOUT" is too generic.  SDHCI_OMAP_TIMEOUT would be better.
> 
> okay.
>>
>>> +
>>> +#define SYSCTL_CLKD_MAX		0x3FF
>>> +
>>> +#define IOV_1V8			1800000		/* 180000 uV */
>>> +#define IOV_3V0			3000000		/* 300000 uV */
>>> +#define IOV_3V3			3300000		/* 330000 uV */
>>> +
>>> +struct sdhci_omap_data {
>>> +	u32 offset;
>>> +};
>>> +
>>> +struct sdhci_omap_host {
>>> +	void __iomem		*base;
>>> +	struct device		*dev;
>>> +	struct	regulator	*pbias;
>>> +	bool			pbias_enabled;
>>> +	struct sdhci_host	*host;
>>> +	unsigned int		flags;
>>> +
>>> +#define SDHCI_OMAP_SUPPORTS_DUAL_VOLT	BIT(0)
>>> +#define SDHCI_OMAP_NO_1_8_V		BIT(1)
>>> +
>>> +	u8			power_mode;
>>> +	bool			io_3_3v_support;
>>> +};
>>> +
> .
> .
> <<snip>>
> .
> .
>>> +static int sdhci_omap_probe(struct platform_device *pdev)
>>> +{
>>> +	int ret;
>>> +	u32 offset;
>>> +	struct device *dev = &pdev->dev;
>>> +	struct sdhci_host *host;
>>> +	struct sdhci_pltfm_host *pltfm_host;
>>> +	struct sdhci_omap_host *omap_host;
>>> +	struct mmc_host *mmc;
>>> +	const struct of_device_id *match;
>>> +	struct sdhci_omap_data *data;
>>> +
>>> +	match = of_match_device(omap_sdhci_match, dev);
>>> +	if (!match)
>>> +		return -EINVAL;
>>> +
>>> +	data = (struct sdhci_omap_data *)match->data;
>>> +	if (!data) {
>>> +		dev_err(dev, "no sdhci omap data\n");
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	offset = data->offset;
>>> +
>>> +	host = sdhci_pltfm_init(pdev, &sdhci_omap_pdata,
>>> +				sizeof(*omap_host));
>>> +	if (IS_ERR(host)) {
>>> +		dev_err(dev, "Failed sdhci_pltfm_init\n");
>>> +		return PTR_ERR(host);
>>> +	}
>>> +
>>> +	pltfm_host = sdhci_priv(host);
>>> +	omap_host = sdhci_pltfm_priv(pltfm_host);
>>> +	omap_host->host = host;
>>> +	omap_host->base = host->ioaddr;
>>> +	omap_host->dev = dev;
>>> +	sdhci_omap_get_of_property(omap_host);
>>> +
>>> +	host->ioaddr += offset;
>>> +
>>> +	mmc = host->mmc;
>>> +	ret = mmc_of_parse(mmc);
>>> +	if (ret)
>>> +		goto err_of_parse;
>>> +
>>> +	pltfm_host->clk = devm_clk_get(dev, "fck");
>>> +	if (IS_ERR(pltfm_host->clk)) {
>>> +		ret = PTR_ERR(pltfm_host->clk);
>>> +		goto err_of_parse;
>>> +	}
>>> +
>>> +	ret = clk_set_rate(pltfm_host->clk, mmc->f_max);
>>> +	if (ret) {
>>> +		dev_err(dev, "failed to set clock to %d\n", mmc->f_max);
>>> +		goto err_of_parse;
>>> +	}
>>> +
>>> +	omap_host->pbias = devm_regulator_get_optional(dev, "pbias");
>>> +	if (IS_ERR(omap_host->pbias)) {
>>> +		ret = PTR_ERR(omap_host->pbias);
>>> +		if (ret != -ENODEV) {
>>> +			dev_err(dev, "CONFIG_REGULATOR_PBIAS not enabled??\n");
>>> +			return ret;
>>> +		}
>>> +		dev_dbg(dev, "unable to get pbias regulator %ld\n",
>>> +			PTR_ERR(omap_host->pbias));
>>> +	} else {
>>> +		if (regulator_is_supported_voltage(omap_host->pbias, IOV_3V3,
>>> +						   IOV_3V3))
>>> +			omap_host->io_3_3v_support = true;
>>> +		dev_vdbg(dev, "Max PBIAS support Voltage: %dmv\n",
>>> +			 omap_host->io_3_3v_support ? IOV_3V3 : IOV_3V0);
>>> +	}
>>> +	omap_host->pbias_enabled = false;
>>> +
>>> +	pm_runtime_enable(dev);
>>> +	ret = pm_runtime_get_sync(dev);
>>
>> What are you trying to do with runtime pm?  There don't seem to be any pm
>> callbacks, so does this do anything?
> 
> yeah, pm_runtime_get_sync enables the functional clock and also configures the
> SYSCONFIG regiters present in the controller. It gets the details for
> configuring those from arch/arm/mach-omap2/omap_hwmod_*

You mean it will do those things when you add the callbacks?  I would prefer
you leave out runtime pm until you can add the callbacks as well.

Re: [RFC PATCH 1/7] mmc: sdhci: Add quirk to indicate no CRC stripping in MMC_RSP_136

[Kishon Vijay Abraham I]

On Thursday 17 August 2017 12:01 PM, Adrian Hunter wrote:
> On 17/08/17 08:20, Kishon Vijay Abraham I wrote:
>> Hi Adrian,
>>
>> On Tuesday 15 August 2017 12:57 PM, Adrian Hunter wrote:
>>> On 07/08/17 19:01, Kishon Vijay Abraham I wrote:
>>>> TI's implementation of sdhci controller used in DRA7 SoC's doesn't
>>>> strip CRC in responses with length 136 bits. Add quirk to indicate
>>>> the controller does not strip CRC in MMC_RSP_136. If this quirk is
>>>> set sdhci library shouldn't shift the response present in
>>>> SDHCI_RESPONSE register.
>>>>
>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>>> ---
>>>>  drivers/mmc/host/sdhci.c | 27 +++++++++++++++++++--------
>>>>  drivers/mmc/host/sdhci.h |  2 ++
>>>>  2 files changed, 21 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>>> index ecd0d4350e8a..ece3751d2a25 100644
>>>> --- a/drivers/mmc/host/sdhci.c
>>>> +++ b/drivers/mmc/host/sdhci.c
>>>> @@ -1182,14 +1182,25 @@ static void sdhci_finish_command(struct sdhci_host *host)
>>>>  
>>>>  	if (cmd->flags & MMC_RSP_PRESENT) {
>>>>  		if (cmd->flags & MMC_RSP_136) {
>>>> -			/* CRC is stripped so we need to do some shifting. */
>>>> -			for (i = 0;i < 4;i++) {
>>>> -				cmd->resp[i] = sdhci_readl(host,
>>>> -					SDHCI_RESPONSE + (3-i)*4) << 8;
>>>> -				if (i != 3)
>>>> -					cmd->resp[i] |=
>>>> -						sdhci_readb(host,
>>>> -						SDHCI_RESPONSE + (3-i)*4-1);
>>>> +			if (!(host->quirks2 & SDHCI_QUIRK2_NO_CRC_STRIPPING)) {
>>>
>>> This is about the 136-bit response so let's put that in the quirk name.  How about SDHCI_QUIRK2_RSP_136_HAS_CRC
>>
>> Since omap seems to be the only platform that doesn't have CRC, I prefer adding
>> SDHCI_QUIRK2_RSP_136_NO_CRC. That way we can add the quirk only in sdhci-omap
>> instead of all the existing sdhci drivers.
> 
> Except that it is the way I described.  All the others strip the CRC and we
> have to shift to pretend it is still there.

right. I made a totally bogus statement.

Thanks
Kishon

Re: [RFC PATCH 6/7] mmc: sdhci-omap: Add OMAP SDHCI driver

[Kishon Vijay Abraham I]

Hi Adrian,

On Thursday 17 August 2017 12:13 PM, Adrian Hunter wrote:
> On 17/08/17 08:57, Kishon Vijay Abraham I wrote:
>> Hi Adrian,
>>
>> On Tuesday 15 August 2017 01:52 PM, Adrian Hunter wrote:
>>> On 07/08/17 19:01, Kishon Vijay Abraham I wrote:
>>>> Create a new sdhci-omap driver to configure the eMMC/SD/SDIO controller
>>>> in TI's OMAP SoCs making use of the SDHCI core library. For OMAP specific
>>>> configurations, populate sdhci_ops with OMAP specific callbacks and use
>>>> SDHCI quirks.
>>>> Enable only high speed mode for both SD and eMMC here and add other
>>>> UHS mode support later.
>>>
>>> I only have a couple of comments below.
>>>
>>>>
>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>>> ---
>>>>  drivers/mmc/host/Kconfig      |  12 +
>>>>  drivers/mmc/host/Makefile     |   1 +
>>>>  drivers/mmc/host/sdhci-omap.c | 593 ++++++++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 606 insertions(+)
>>>>  create mode 100644 drivers/mmc/host/sdhci-omap.c
>>>>
>>>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>>>> index 5755b69f2f72..9aa7e5ce43ba 100644
>>>> --- a/drivers/mmc/host/Kconfig
>>>> +++ b/drivers/mmc/host/Kconfig
>>>> @@ -871,3 +871,15 @@ config MMC_SDHCI_XENON
>>>>  	  This selects Marvell Xenon eMMC/SD/SDIO SDHCI.
>>>>  	  If you have a controller with this interface, say Y or M here.
>>>>  	  If unsure, say N.
>>>> +
>>>> +config MMC_SDHCI_OMAP
>>>> +	tristate "TI SDHCI Controller Support"
>>>> +	depends on MMC_SDHCI_PLTFM
>>>> +	help
>>>> +	  This selects the Secure Digital Host Controller Interface (SDHCI)
>>>> +	  support present in TI's DRA7 SOCs. The controller supports
>>>> +	  SD/MMC/SDIO devices.
>>>> +
>>>> +	  If you have a controller with this interface, say Y or M here.
>>>> +
>>>> +	  If unsure, say N.
>>>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>>>> index 4d4547116311..30fb8b0fd0e1 100644
>>>> --- a/drivers/mmc/host/Makefile
>>>> +++ b/drivers/mmc/host/Makefile
>>>> @@ -82,6 +82,7 @@ obj-$(CONFIG_MMC_SDHCI_MSM)		+= sdhci-msm.o
>>>>  obj-$(CONFIG_MMC_SDHCI_ST)		+= sdhci-st.o
>>>>  obj-$(CONFIG_MMC_SDHCI_MICROCHIP_PIC32)	+= sdhci-pic32.o
>>>>  obj-$(CONFIG_MMC_SDHCI_BRCMSTB)		+= sdhci-brcmstb.o
>>>> +obj-$(CONFIG_MMC_SDHCI_OMAP)		+= sdhci-omap.o
>>>>  
>>>>  ifeq ($(CONFIG_CB710_DEBUG),y)
>>>>  	CFLAGS-cb710-mmc	+= -DDEBUG
>>>> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
>>>> new file mode 100644
>>>> index 000000000000..c067c428a0cd
>>>> --- /dev/null
>>>> +++ b/drivers/mmc/host/sdhci-omap.c
>>>> @@ -0,0 +1,593 @@
>>>> +/*
>>>> + * SDHCI Controller driver for TI's OMAP SoCs
>>>> + *
>>>> + * Copyright (C) 2016-2017 Texas Instruments Incorporated - http://www.ti.com
>>>> + *
>>>> + * Authors: Kishon Vijay Abraham I <kishon@ti.com>
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or modify
>>>> + * it under the terms of the GNU General Public License version 2 as
>>>> + * published by the Free Software Foundation.
>>>> + */
>>>> +
>>>> +#include <linux/delay.h>
>>>> +#include <linux/mmc/slot-gpio.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/of.h>
>>>> +#include <linux/of_device.h>
>>>> +#include <linux/platform_device.h>
>>>> +#include <linux/pm_runtime.h>
>>>> +#include <linux/regulator/consumer.h>
>>>> +
>>>> +#include "sdhci-pltfm.h"
>>>> +
>>>> +#define SDHCI_OMAP_CON		0x12c
>>>> +#define CON_DW8			BIT(5)
>>>> +#define CON_DMA_MASTER		BIT(20)
>>>> +#define CON_INIT		BIT(1)
>>>> +#define CON_OD			BIT(0)
>>>> +
>>>> +#define SDHCI_OMAP_CMD		0x20c
>>>> +
>>>> +#define SDHCI_OMAP_HCTL		0x228
>>>> +#define HCTL_SDBP		BIT(8)
>>>> +#define HCTL_SDVS_SHIFT		9
>>>> +#define HCTL_SDVS_MASK		(0x7 << HCTL_SDVS_SHIFT)
>>>> +#define HCTL_SDVS_33		(0x7 << HCTL_SDVS_SHIFT)
>>>> +#define HCTL_SDVS_30		(0x6 << HCTL_SDVS_SHIFT)
>>>> +#define HCTL_SDVS_18		(0x5 << HCTL_SDVS_SHIFT)
>>>> +
>>>> +#define SDHCI_OMAP_SYSCTL	0x22c
>>>> +#define SYSCTL_CEN		BIT(2)
>>>> +#define SYSCTL_CLKD_SHIFT	6
>>>> +#define SYSCTL_CLKD_MASK	0x3ff
>>>> +
>>>> +#define SDHCI_OMAP_STAT		0x230
>>>> +
>>>> +#define SDHCI_OMAP_IE		0x234
>>>> +#define INT_CC_EN		BIT(0)
>>>> +
>>>> +#define SDHCI_OMAP_AC12		0x23c
>>>> +#define AC12_V1V8_SIGEN		BIT(19)
>>>> +
>>>> +#define SDHCI_OMAP_CAPA		0x240
>>>> +#define CAPA_VS33		BIT(24)
>>>> +#define CAPA_VS30		BIT(25)
>>>> +#define CAPA_VS18		BIT(26)
>>>> +
>>>> +#define MMC_TIMEOUT		1		/* 1 msec */
>>>
>>> This name "MMC_TIMEOUT" is too generic.  SDHCI_OMAP_TIMEOUT would be better.
>>
>> okay.
>>>
>>>> +
>>>> +#define SYSCTL_CLKD_MAX		0x3FF
>>>> +
>>>> +#define IOV_1V8			1800000		/* 180000 uV */
>>>> +#define IOV_3V0			3000000		/* 300000 uV */
>>>> +#define IOV_3V3			3300000		/* 330000 uV */
>>>> +
>>>> +struct sdhci_omap_data {
>>>> +	u32 offset;
>>>> +};
>>>> +
>>>> +struct sdhci_omap_host {
>>>> +	void __iomem		*base;
>>>> +	struct device		*dev;
>>>> +	struct	regulator	*pbias;
>>>> +	bool			pbias_enabled;
>>>> +	struct sdhci_host	*host;
>>>> +	unsigned int		flags;
>>>> +
>>>> +#define SDHCI_OMAP_SUPPORTS_DUAL_VOLT	BIT(0)
>>>> +#define SDHCI_OMAP_NO_1_8_V		BIT(1)
>>>> +
>>>> +	u8			power_mode;
>>>> +	bool			io_3_3v_support;
>>>> +};
>>>> +
>> .
>> .
>> <<snip>>
>> .
>> .
>>>> +static int sdhci_omap_probe(struct platform_device *pdev)
>>>> +{
>>>> +	int ret;
>>>> +	u32 offset;
>>>> +	struct device *dev = &pdev->dev;
>>>> +	struct sdhci_host *host;
>>>> +	struct sdhci_pltfm_host *pltfm_host;
>>>> +	struct sdhci_omap_host *omap_host;
>>>> +	struct mmc_host *mmc;
>>>> +	const struct of_device_id *match;
>>>> +	struct sdhci_omap_data *data;
>>>> +
>>>> +	match = of_match_device(omap_sdhci_match, dev);
>>>> +	if (!match)
>>>> +		return -EINVAL;
>>>> +
>>>> +	data = (struct sdhci_omap_data *)match->data;
>>>> +	if (!data) {
>>>> +		dev_err(dev, "no sdhci omap data\n");
>>>> +		return -EINVAL;
>>>> +	}
>>>> +
>>>> +	offset = data->offset;
>>>> +
>>>> +	host = sdhci_pltfm_init(pdev, &sdhci_omap_pdata,
>>>> +				sizeof(*omap_host));
>>>> +	if (IS_ERR(host)) {
>>>> +		dev_err(dev, "Failed sdhci_pltfm_init\n");
>>>> +		return PTR_ERR(host);
>>>> +	}
>>>> +
>>>> +	pltfm_host = sdhci_priv(host);
>>>> +	omap_host = sdhci_pltfm_priv(pltfm_host);
>>>> +	omap_host->host = host;
>>>> +	omap_host->base = host->ioaddr;
>>>> +	omap_host->dev = dev;
>>>> +	sdhci_omap_get_of_property(omap_host);
>>>> +
>>>> +	host->ioaddr += offset;
>>>> +
>>>> +	mmc = host->mmc;
>>>> +	ret = mmc_of_parse(mmc);
>>>> +	if (ret)
>>>> +		goto err_of_parse;
>>>> +
>>>> +	pltfm_host->clk = devm_clk_get(dev, "fck");
>>>> +	if (IS_ERR(pltfm_host->clk)) {
>>>> +		ret = PTR_ERR(pltfm_host->clk);
>>>> +		goto err_of_parse;
>>>> +	}
>>>> +
>>>> +	ret = clk_set_rate(pltfm_host->clk, mmc->f_max);
>>>> +	if (ret) {
>>>> +		dev_err(dev, "failed to set clock to %d\n", mmc->f_max);
>>>> +		goto err_of_parse;
>>>> +	}
>>>> +
>>>> +	omap_host->pbias = devm_regulator_get_optional(dev, "pbias");
>>>> +	if (IS_ERR(omap_host->pbias)) {
>>>> +		ret = PTR_ERR(omap_host->pbias);
>>>> +		if (ret != -ENODEV) {
>>>> +			dev_err(dev, "CONFIG_REGULATOR_PBIAS not enabled??\n");
>>>> +			return ret;
>>>> +		}
>>>> +		dev_dbg(dev, "unable to get pbias regulator %ld\n",
>>>> +			PTR_ERR(omap_host->pbias));
>>>> +	} else {
>>>> +		if (regulator_is_supported_voltage(omap_host->pbias, IOV_3V3,
>>>> +						   IOV_3V3))
>>>> +			omap_host->io_3_3v_support = true;
>>>> +		dev_vdbg(dev, "Max PBIAS support Voltage: %dmv\n",
>>>> +			 omap_host->io_3_3v_support ? IOV_3V3 : IOV_3V0);
>>>> +	}
>>>> +	omap_host->pbias_enabled = false;
>>>> +
>>>> +	pm_runtime_enable(dev);
>>>> +	ret = pm_runtime_get_sync(dev);
>>>
>>> What are you trying to do with runtime pm?  There don't seem to be any pm
>>> callbacks, so does this do anything?
>>
>> yeah, pm_runtime_get_sync enables the functional clock and also configures the
>> SYSCONFIG regiters present in the controller. It gets the details for
>> configuring those from arch/arm/mach-omap2/omap_hwmod_*
> 
> You mean it will do those things when you add the callbacks?  I would prefer
> you leave out runtime pm until you can add the callbacks as well.

No. Generally in callbacks, 'optional' functional clocks are enabled. But main
functional clock and interface clocks are enabled in pm_runtime_get_sync.
Without pm_runtime_get_sync, we won't be able to access controller registers.

Thanks
Kishon

Re: [RFC PATCH 6/7] mmc: sdhci-omap: Add OMAP SDHCI driver

[Adrian Hunter]

On 17/08/17 10:59, Kishon Vijay Abraham I wrote:
> Hi Adrian,
> 
> On Thursday 17 August 2017 12:13 PM, Adrian Hunter wrote:
>> On 17/08/17 08:57, Kishon Vijay Abraham I wrote:
>>> Hi Adrian,
>>>
>>> On Tuesday 15 August 2017 01:52 PM, Adrian Hunter wrote:
>>>> On 07/08/17 19:01, Kishon Vijay Abraham I wrote:

<SNIP>

>>>>> +	pm_runtime_enable(dev);
>>>>> +	ret = pm_runtime_get_sync(dev);
>>>>
>>>> What are you trying to do with runtime pm?  There don't seem to be any pm
>>>> callbacks, so does this do anything?
>>>
>>> yeah, pm_runtime_get_sync enables the functional clock and also configures the
>>> SYSCONFIG regiters present in the controller. It gets the details for
>>> configuring those from arch/arm/mach-omap2/omap_hwmod_*
>>
>> You mean it will do those things when you add the callbacks?  I would prefer
>> you leave out runtime pm until you can add the callbacks as well.
> 
> No. Generally in callbacks, 'optional' functional clocks are enabled. But main
> functional clock and interface clocks are enabled in pm_runtime_get_sync.
> Without pm_runtime_get_sync, we won't be able to access controller registers.

So that is done by the pm domain?  Please add comments to the code to explain.

Re: [RFC PATCH 5/7] dt-bindings: ti-omap-hsmmc: Document new compatible for sdhci omap

[Tony Lindgren]

* Kishon Vijay Abraham I <kishon@ti.com> [170816 22:44]:
> Hi Tony,
> 
> On Thursday 10 August 2017 03:42 AM, Tony Lindgren wrote:
> > * Kishon Vijay Abraham I <kishon@ti.com> [170807 09:03]:
> >> Document the new compatible string "ti,dra7-sdhci" to be used for
> >> MMC controllers in DRA7 and DRA72 SoCs.
> > 
> > I wonder if this should really be documented for sdhci
> > instead of ti-omap-hsmmc.txt?
> 
> hmm.. yeah, having a separate binding document for sdhci would make the binding
> really clean and we also don't have to carry legacy binding code. But we'll be
> never able to remove omap-hsmmc driver then and end up maintaining 2 drivers
> for the same controller.

Well I think it's best to be able to enable sdhci one board at
a time after testing. Then eventually when fully supported, we
can just have sdhci also parse the legacy omap binding and just
drop the omap-hsmmc driver.

Regards,

Tony

Re: [RFC PATCH 6/7] mmc: sdhci-omap: Add OMAP SDHCI driver

[Kishon Vijay Abraham I]

Adrian,

On Thursday 17 August 2017 01:53 PM, Adrian Hunter wrote:
> On 17/08/17 10:59, Kishon Vijay Abraham I wrote:
>> Hi Adrian,
>>
>> On Thursday 17 August 2017 12:13 PM, Adrian Hunter wrote:
>>> On 17/08/17 08:57, Kishon Vijay Abraham I wrote:
>>>> Hi Adrian,
>>>>
>>>> On Tuesday 15 August 2017 01:52 PM, Adrian Hunter wrote:
>>>>> On 07/08/17 19:01, Kishon Vijay Abraham I wrote:
> 
> <SNIP>
> 
>>>>>> +	pm_runtime_enable(dev);
>>>>>> +	ret = pm_runtime_get_sync(dev);
>>>>>
>>>>> What are you trying to do with runtime pm?  There don't seem to be any pm
>>>>> callbacks, so does this do anything?
>>>>
>>>> yeah, pm_runtime_get_sync enables the functional clock and also configures the
>>>> SYSCONFIG regiters present in the controller. It gets the details for
>>>> configuring those from arch/arm/mach-omap2/omap_hwmod_*
>>>
>>> You mean it will do those things when you add the callbacks?  I would prefer
>>> you leave out runtime pm until you can add the callbacks as well.
>>
>> No. Generally in callbacks, 'optional' functional clocks are enabled. But main
>> functional clock and interface clocks are enabled in pm_runtime_get_sync.
>> Without pm_runtime_get_sync, we won't be able to access controller registers.
> 
> So that is done by the pm domain?  Please add comments to the code to explain.

right, it's done by omap_device_pm_domain.
Invoking pm_runtime_get_sync of a platform device in OMAP platforms will
eventually come to omap_hwmod_enable where main functional clock, interface
clock and sysconfig are programmed.

Thanks
Kishon