The following patches add driver support for MMCSD on TI's AM654
platforms.
Previously I had added the support to sdhci-of-arasan driver with
a separate phy driver[1]. Since then it has turned out that tuning
operation (for HS200, HS400 and SDR104 speed modes) will require
configuration of phy registers. This completely breaks the model of
the sdhci-of-arasan driver which relies on a separate driver to
configure the phy register space.
Because of this, I am creating a new driver with both the sdhci and
phy register spaces. This helps me use the phy registers in a future
patch that adds tuning support.
DT patches will be posted in a separate series.
[1] driver and phy patches posted before
https://patchwork.kernel.org/project/linux-mmc/list/?series=26623
Faiz Abbas (3):
dt-bindings: mmc: sdhci-am654: Document bindings for the host
controllers on TI's AM654 SOCs
dt-bindings: mmc: sdhci-of-arasan: Add deprecated message for am654
mmc: sdhci_am654: Add Initial Support for AM654 SDHCI driver
.../devicetree/bindings/mmc/arasan,sdhci.txt | 4 +
.../devicetree/bindings/mmc/sdhci-am654.txt | 23 ++
drivers/mmc/host/Kconfig | 12 +
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/sdhci-of-arasan.c | 46 ---
drivers/mmc/host/sdhci_am654.c | 366 ++++++++++++++++++
6 files changed, 406 insertions(+), 46 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-am654.txt
create mode 100644 drivers/mmc/host/sdhci_am654.c
--
2.19.2
Add binding documentation for mmc host controllers present on TI's AM654
SOCs
Signed-off-by: Faiz Abbas <[email protected]>
---
.../devicetree/bindings/mmc/sdhci-am654.txt | 37 +++++++++++++++++++
1 file changed, 37 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-am654.txt
diff --git a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
new file mode 100644
index 000000000000..862b93051bfe
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
@@ -0,0 +1,37 @@
+Device Tree Bindings for the SDHCI Controllers present on TI's AM654 SOCs
+
+The bindings follow the mmc[1], clock[2] and interrupt[3] bindings.
+Only deviations are documented here.
+
+ [1] Documentation/devicetree/bindings/mmc/mmc.txt
+ [2] Documentation/devicetree/bindings/clock/clock-bindings.txt
+ [3] Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+
+Required Properties:
+ - compatible: should be "ti,am654-sdhci-5.1"
+ - reg: Must be two entries.
+ - The first should be the sdhci register space
+ - The second should the subsystem/phy register space
+ - clocks: Handles to the clock inputs.
+ - clock-names: Tuple including "clk_xin" and "clk_ahb"
+ - interrupts: Interrupt specifiers
+ - ti,otap-del-sel: Output Tap Delay select
+ - ti,trm-icp: DLL trim select
+ - ti,driver-strength-ohm: driver strength in ohms.
+ Valid values are 33, 40, 50, 66 and 100 ohms.
+
+Example:
+
+ sdhci0: sdhci@4f80000 {
+ compatible = "ti,am654-sdhci-5.1";
+ reg = <0x0 0x4f80000 0x0 0x260>, <0x0 0x4f90000 0x0 0x134>;
+ power-domains = <&k3_pds 47>;
+ clocks = <&k3_clks 47 0>, <&k3_clks 47 1>;
+ clock-names = "clk_ahb", "clk_xin";
+ interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
+ sdhci-caps-mask = <0x80000007 0x0>;
+ mmc-ddr-1_8v;
+ ti,otap-del-sel = <0x2>;
+ ti,trm-icp = <0x8>;
+ status = "disabled";
+ };
--
2.19.2
The "ti,am654-sdhci-5.1" binding has been moved to a new driver. Indicate
this by a deprecated message.
Signed-off-by: Faiz Abbas <[email protected]>
---
Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
index e2effe17f05e..1edbb049cccb 100644
--- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
@@ -16,6 +16,10 @@ Required Properties:
- "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1": rk3399 eMMC PHY
For this device it is strongly suggested to include arasan,soc-ctl-syscon.
- "ti,am654-sdhci-5.1", "arasan,sdhci-5.1": TI AM654 MMC PHY
+ Note: This binding has been deprecated and moved to [5].
+
+ [5] Documentation/devicetree/bindings/mmc/sdhci-am654.txt
+
- reg: From mmc bindings: Register location and length.
- clocks: From clock bindings: Handles to clock inputs.
- clock-names: From clock bindings: Tuple including "clk_xin" and "clk_ahb"
--
2.19.2
The host controllers on TI's AM654 SOCs are not compatible with
the phy and consumer model of the sdhci-of-arasan driver. It turns out
that for optimal operation at higher speeds, a special tuning procedure
needs to be implemented which involves configuration of platform
specific phy registers.
Therefore, branch out to a new sdhci_am654 driver and add the phy
register space with all phy configurations to it. Populate AM654
specific callbacks to sdhci_ops and add SDHCI_QUIRKS wherever
applicable.
Only add support for upto High Speed for SD card and upto DDR52 speed
mode for eMMC. Higher speeds will be added in subsequent patches.
Signed-off-by: Faiz Abbas <[email protected]>
---
drivers/mmc/host/Kconfig | 12 +
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/sdhci-of-arasan.c | 46 ----
drivers/mmc/host/sdhci_am654.c | 376 +++++++++++++++++++++++++++++
4 files changed, 389 insertions(+), 46 deletions(-)
create mode 100644 drivers/mmc/host/sdhci_am654.c
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 1b58739d9744..cfb2eb1a2c32 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -977,3 +977,15 @@ config MMC_SDHCI_OMAP
If you have a controller with this interface, say Y or M here.
If unsure, say N.
+
+config MMC_SDHCI_AM654
+ tristate "Support for the SDHCI Controller in TI's AM654 SOCs"
+ depends on MMC_SDHCI_PLTFM && OF
+ help
+ This selects the Secure Digital Host Controller Interface (SDHCI)
+ support present in TI's AM654 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 720d37777098..5c7770edc431 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_MMC_SDHCI_S3C) += sdhci-s3c.o
obj-$(CONFIG_MMC_SDHCI_SIRF) += sdhci-sirf.o
obj-$(CONFIG_MMC_SDHCI_F_SDH30) += sdhci_f_sdh30.o
obj-$(CONFIG_MMC_SDHCI_SPEAR) += sdhci-spear.o
+obj-$(CONFIG_MMC_SDHCI_AM654) += sdhci_am654.o
obj-$(CONFIG_MMC_WBSD) += wbsd.o
obj-$(CONFIG_MMC_AU1X) += au1xmmc.o
obj-$(CONFIG_MMC_MTK) += mtk-sd.o
diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 142c4b802f31..c9e3e050ccc8 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -231,25 +231,6 @@ static void sdhci_arasan_set_clock(struct sdhci_host *host, unsigned int clock)
}
}
-static void sdhci_arasan_am654_set_clock(struct sdhci_host *host,
- unsigned int clock)
-{
- struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
- struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
-
- if (sdhci_arasan->is_phy_on) {
- phy_power_off(sdhci_arasan->phy);
- sdhci_arasan->is_phy_on = false;
- }
-
- sdhci_set_clock(host, clock);
-
- if (clock > PHY_CLK_TOO_SLOW_HZ) {
- phy_power_on(sdhci_arasan->phy);
- sdhci_arasan->is_phy_on = true;
- }
-}
-
static void sdhci_arasan_hs400_enhanced_strobe(struct mmc_host *mmc,
struct mmc_ios *ios)
{
@@ -335,29 +316,6 @@ static struct sdhci_arasan_of_data sdhci_arasan_data = {
.pdata = &sdhci_arasan_pdata,
};
-static const struct sdhci_ops sdhci_arasan_am654_ops = {
- .set_clock = sdhci_arasan_am654_set_clock,
- .get_max_clock = sdhci_pltfm_clk_get_max_clock,
- .get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
- .set_bus_width = sdhci_set_bus_width,
- .reset = sdhci_arasan_reset,
- .set_uhs_signaling = sdhci_set_uhs_signaling,
-};
-
-static const struct sdhci_pltfm_data sdhci_arasan_am654_pdata = {
- .ops = &sdhci_arasan_am654_ops,
- .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
- SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
- SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
- .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
- SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN |
- SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400,
-};
-
-static const struct sdhci_arasan_of_data sdhci_arasan_am654_data = {
- .pdata = &sdhci_arasan_am654_pdata,
-};
-
static u32 sdhci_arasan_cqhci_irq(struct sdhci_host *host, u32 intmask)
{
int cmd_error = 0;
@@ -520,10 +478,6 @@ static const struct of_device_id sdhci_arasan_of_match[] = {
.compatible = "rockchip,rk3399-sdhci-5.1",
.data = &sdhci_arasan_rk3399_data,
},
- {
- .compatible = "ti,am654-sdhci-5.1",
- .data = &sdhci_arasan_am654_data,
- },
/* Generic compatible below here */
{
.compatible = "arasan,sdhci-8.9a",
diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
new file mode 100644
index 000000000000..6e5be12d5946
--- /dev/null
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -0,0 +1,376 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * sdhci_am654.c - SDHCI driver for TI's AM654 SOCs
+ *
+ * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com
+ *
+ */
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+#include "sdhci-pltfm.h"
+
+/* CTL_CFG Registers */
+#define CTL_CFG_2 0x14
+
+#define SLOTTYPE_MASK GENMASK(31, 30)
+#define SLOTTYPE_EMBEDDED BIT(30)
+
+/* PHY Registers */
+#define PHY_CTRL1 0x100
+#define PHY_CTRL2 0x104
+#define PHY_CTRL3 0x108
+#define PHY_CTRL4 0x10C
+#define PHY_CTRL5 0x110
+#define PHY_CTRL6 0x114
+#define PHY_STAT1 0x130
+#define PHY_STAT2 0x134
+
+#define IOMUX_ENABLE_SHIFT 31
+#define IOMUX_ENABLE_MASK BIT(IOMUX_ENABLE_SHIFT)
+#define OTAPDLYENA_SHIFT 20
+#define OTAPDLYENA_MASK BIT(OTAPDLYENA_SHIFT)
+#define OTAPDLYSEL_SHIFT 12
+#define OTAPDLYSEL_MASK GENMASK(15, 12)
+#define STRBSEL_SHIFT 24
+#define STRBSEL_MASK GENMASK(27, 24)
+#define SEL50_SHIFT 8
+#define SEL50_MASK BIT(SEL50_SHIFT)
+#define SEL100_SHIFT 9
+#define SEL100_MASK BIT(SEL100_SHIFT)
+#define DLL_TRIM_ICP_SHIFT 4
+#define DLL_TRIM_ICP_MASK GENMASK(7, 4)
+#define DR_TY_SHIFT 20
+#define DR_TY_MASK GENMASK(22, 20)
+#define ENDLL_SHIFT 1
+#define ENDLL_MASK BIT(ENDLL_SHIFT)
+#define DLLRDY_SHIFT 0
+#define DLLRDY_MASK BIT(DLLRDY_SHIFT)
+#define PDB_SHIFT 0
+#define PDB_MASK BIT(PDB_SHIFT)
+#define CALDONE_SHIFT 1
+#define CALDONE_MASK BIT(CALDONE_SHIFT)
+#define RETRIM_SHIFT 17
+#define RETRIM_MASK BIT(RETRIM_SHIFT)
+
+#define DRIVER_STRENGTH_50_OHM 0x0
+#define DRIVER_STRENGTH_33_OHM 0x1
+#define DRIVER_STRENGTH_66_OHM 0x2
+#define DRIVER_STRENGTH_100_OHM 0x3
+#define DRIVER_STRENGTH_40_OHM 0x4
+
+#define CLOCK_TOO_SLOW_HZ 400000
+
+static struct regmap_config sdhci_am654_regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .fast_io = true,
+};
+
+struct sdhci_am654_data {
+ struct regmap *base;
+ struct clk *clk_ahb;
+ int otap_del_sel;
+ int trm_icp;
+ int drv_strength;
+ bool dll_on;
+};
+
+static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
+ int sel50, sel100;
+ u32 mask, val;
+ int ret;
+
+ if (sdhci_am654->dll_on) {
+ regmap_update_bits(sdhci_am654->base, PHY_CTRL1,
+ ENDLL_MASK, 0);
+
+ sdhci_am654->dll_on = false;
+ }
+
+ sdhci_set_clock(host, clock);
+
+ if (clock > CLOCK_TOO_SLOW_HZ) {
+ /* Setup DLL Output TAP delay */
+ mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
+ val = (1 << OTAPDLYENA_SHIFT) |
+ (sdhci_am654->otap_del_sel << OTAPDLYSEL_SHIFT);
+ regmap_update_bits(sdhci_am654->base, PHY_CTRL4,
+ mask, val);
+ switch (clock) {
+ case 200000000:
+ sel50 = 0;
+ sel100 = 0;
+ break;
+ case 100000000:
+ sel50 = 0;
+ sel100 = 1;
+ break;
+ default:
+ sel50 = 1;
+ sel100 = 0;
+ }
+
+ /* Configure PHY DLL frequency */
+ mask = SEL50_MASK | SEL100_MASK;
+ val = (sel50 << SEL50_SHIFT) | (sel100 << SEL100_SHIFT);
+ regmap_update_bits(sdhci_am654->base, PHY_CTRL5,
+ mask, val);
+ /* Configure DLL TRIM */
+ mask = DLL_TRIM_ICP_MASK;
+ val = sdhci_am654->trm_icp << DLL_TRIM_ICP_SHIFT;
+
+ /* Configure DLL driver strength */
+ mask |= DR_TY_MASK;
+ val |= sdhci_am654->drv_strength << DR_TY_SHIFT;
+ regmap_update_bits(sdhci_am654->base, PHY_CTRL1,
+ mask, val);
+ /* Enable DLL */
+ regmap_update_bits(sdhci_am654->base, PHY_CTRL1,
+ ENDLL_MASK, 0x1 << ENDLL_SHIFT);
+ /*
+ * Poll for DLL ready. Use a one second timeout.
+ * Works in all experiments done so far
+ */
+ ret = regmap_read_poll_timeout(sdhci_am654->base,
+ PHY_STAT1, val,
+ val & DLLRDY_MASK,
+ 1000, 1000000);
+
+ sdhci_am654->dll_on = true;
+ }
+}
+
+
+static void sdhci_am654_set_power(struct sdhci_host *host, unsigned char mode,
+ unsigned short vdd)
+{
+ if (!IS_ERR(host->mmc->supply.vmmc)) {
+ struct mmc_host *mmc = host->mmc;
+
+ mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
+ }
+ sdhci_set_power_noreg(host, mode, vdd);
+}
+
+struct sdhci_ops sdhci_am654_ops = {
+ .get_max_clock = sdhci_pltfm_clk_get_max_clock,
+ .get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
+ .set_uhs_signaling = sdhci_set_uhs_signaling,
+ .set_bus_width = sdhci_set_bus_width,
+ .set_power = sdhci_am654_set_power,
+ .set_clock = sdhci_am654_set_clock,
+ .reset = sdhci_reset,
+};
+
+static const struct sdhci_pltfm_data sdhci_am654_pdata = {
+ .ops = &sdhci_am654_ops,
+ .quirks = SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
+ SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
+ .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
+};
+
+static int sdhci_am654_init(struct sdhci_host *host)
+{
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
+ u32 ctl_cfg_2 = 0;
+ u32 val;
+ int ret;
+
+ regmap_read(sdhci_am654->base, PHY_STAT1, &val);
+ if (~val & CALDONE_MASK) {
+ /* Calibrate IO lines */
+ regmap_update_bits(sdhci_am654->base, PHY_CTRL1,
+ PDB_MASK, PDB_MASK);
+ ret = regmap_read_poll_timeout(sdhci_am654->base, PHY_STAT1,
+ val, val & CALDONE_MASK, 1, 20);
+ if (ret)
+ return ret;
+ }
+
+ /* Enable pins by setting IO mux to 0 */
+ regmap_update_bits(sdhci_am654->base, PHY_CTRL1,
+ IOMUX_ENABLE_MASK, 0);
+
+ /* Set slot type based on SD or eMMC */
+ if (host->mmc->caps & MMC_CAP_NONREMOVABLE)
+ ctl_cfg_2 = SLOTTYPE_EMBEDDED;
+
+ regmap_update_bits(sdhci_am654->base, CTL_CFG_2,
+ ctl_cfg_2, SLOTTYPE_MASK);
+
+ return sdhci_add_host(host);
+}
+
+static int sdhci_am654_get_of_property(struct platform_device *pdev,
+ struct sdhci_am654_data *sdhci_am654)
+{
+ struct device *dev = &pdev->dev;
+ int drv_strength;
+ int ret;
+
+ ret = device_property_read_u32(dev, "ti,trm-icp",
+ &sdhci_am654->trm_icp);
+ if (ret)
+ return ret;
+
+ ret = device_property_read_u32(dev, "ti,otap-del-sel",
+ &sdhci_am654->otap_del_sel);
+ if (ret)
+ return ret;
+
+ ret = device_property_read_u32(dev, "ti,driver-strength-ohm",
+ &drv_strength);
+ if (ret)
+ return ret;
+
+ switch (drv_strength) {
+ case 50:
+ sdhci_am654->drv_strength = DRIVER_STRENGTH_50_OHM;
+ break;
+ case 33:
+ sdhci_am654->drv_strength = DRIVER_STRENGTH_33_OHM;
+ break;
+ case 66:
+ sdhci_am654->drv_strength = DRIVER_STRENGTH_66_OHM;
+ break;
+ case 100:
+ sdhci_am654->drv_strength = DRIVER_STRENGTH_100_OHM;
+ break;
+ case 40:
+ sdhci_am654->drv_strength = DRIVER_STRENGTH_40_OHM;
+ break;
+ default:
+ dev_err(dev, "Invalid driver strength\n");
+ return -EINVAL;
+ }
+
+ sdhci_get_of_property(pdev);
+
+ return 0;
+}
+
+static int sdhci_am654_probe(struct platform_device *pdev)
+{
+ struct sdhci_pltfm_host *pltfm_host;
+ struct sdhci_am654_data *sdhci_am654;
+ struct sdhci_host *host;
+ struct resource *res;
+ struct clk *clk_xin;
+ struct device *dev = &pdev->dev;
+ void __iomem *base;
+ int ret;
+
+ host = sdhci_pltfm_init(pdev, &sdhci_am654_pdata, sizeof(*sdhci_am654));
+ if (IS_ERR(host))
+ return PTR_ERR(host);
+
+ pltfm_host = sdhci_priv(host);
+ sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
+
+ clk_xin = devm_clk_get(dev, "clk_xin");
+ if (IS_ERR(clk_xin)) {
+ dev_err(dev, "clk_xin clock not found.\n");
+ ret = PTR_ERR(clk_xin);
+ goto err_pltfm_free;
+ }
+
+ sdhci_am654->clk_ahb = devm_clk_get(dev, "clk_ahb");
+ if (IS_ERR(sdhci_am654->clk_ahb)) {
+ dev_err(dev, "clk_ahb clock not found.\n");
+ ret = PTR_ERR(sdhci_am654->clk_ahb);
+ goto err_pltfm_free;
+ }
+
+ pltfm_host->clk = clk_xin;
+
+ pm_runtime_enable(dev);
+ ret = pm_runtime_get_sync(dev);
+ if (ret > 0) {
+ pm_runtime_put_noidle(dev);
+ goto pm_runtime_disable;
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(base)) {
+ ret = PTR_ERR(base);
+ goto pm_runtime_put;
+ }
+
+ sdhci_am654->base = devm_regmap_init_mmio(dev, base,
+ &sdhci_am654_regmap_config);
+ if (IS_ERR(sdhci_am654->base)) {
+ dev_err(dev, "Failed to initialize regmap\n");
+ ret = PTR_ERR(sdhci_am654->base);
+ goto pm_runtime_put;
+ }
+
+ ret = sdhci_am654_get_of_property(pdev, sdhci_am654);
+ if (ret)
+ goto pm_runtime_put;
+
+ ret = mmc_of_parse(host->mmc);
+ if (ret) {
+ dev_err(dev, "parsing dt failed (%d)\n", ret);
+ goto pm_runtime_put;
+ }
+
+ ret = sdhci_am654_init(host);
+ if (ret)
+ goto pm_runtime_put;
+
+ return 0;
+
+pm_runtime_put:
+ pm_runtime_put_sync(dev);
+pm_runtime_disable:
+ pm_runtime_disable(dev);
+err_pltfm_free:
+ sdhci_pltfm_free(pdev);
+ return ret;
+}
+
+static int sdhci_am654_remove(struct platform_device *pdev)
+{
+ struct sdhci_host *host = platform_get_drvdata(pdev);
+ int ret;
+
+ sdhci_remove_host(host, true);
+ ret = pm_runtime_put_sync(&pdev->dev);
+ if (ret < 0)
+ return ret;
+
+ pm_runtime_disable(&pdev->dev);
+ sdhci_pltfm_free(pdev);
+
+ return 0;
+}
+
+static const struct of_device_id sdhci_am654_of_match[] = {
+ { .compatible = "ti,am654-sdhci-5.1" },
+ { /* sentinel */ }
+};
+
+static struct platform_driver sdhci_am654_driver = {
+ .driver = {
+ .name = "sdhci-am654",
+ .of_match_table = sdhci_am654_of_match,
+ },
+ .probe = sdhci_am654_probe,
+ .remove = sdhci_am654_remove,
+};
+
+module_platform_driver(sdhci_am654_driver);
+
+MODULE_DESCRIPTION("Driver for SDHCI Controller on TI's AM654 devices");
+MODULE_AUTHOR("Faiz Abbas <[email protected]>");
+MODULE_LICENSE("GPL");
--
2.19.2
On 29. 11. 18 17:15, Faiz Abbas wrote:
> The "ti,am654-sdhci-5.1" binding has been moved to a new driver. Indicate
> this by a deprecated message.
>
> Signed-off-by: Faiz Abbas <[email protected]>
> ---
> Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> index e2effe17f05e..1edbb049cccb 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -16,6 +16,10 @@ Required Properties:
> - "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1": rk3399 eMMC PHY
> For this device it is strongly suggested to include arasan,soc-ctl-syscon.
> - "ti,am654-sdhci-5.1", "arasan,sdhci-5.1": TI AM654 MMC PHY
> + Note: This binding has been deprecated and moved to [5].
> +
> + [5] Documentation/devicetree/bindings/mmc/sdhci-am654.txt
> +
> - reg: From mmc bindings: Register location and length.
> - clocks: From clock bindings: Handles to clock inputs.
> - clock-names: From clock bindings: Tuple including "clk_xin" and "clk_ahb"
>
I would prefer if you can extend commit message by description you have
in cover letter which explains reasons. When this is applied and someone
look at git history none will understand from first read why you have
done that.
Thanks,
Michal
Hi Michal,
On 30/11/18 12:48 PM, Michal Simek wrote:
> On 29. 11. 18 17:15, Faiz Abbas wrote:
>> The "ti,am654-sdhci-5.1" binding has been moved to a new driver. Indicate
>> this by a deprecated message.
>>
>> Signed-off-by: Faiz Abbas <[email protected]>
>> ---
>> Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
>> index e2effe17f05e..1edbb049cccb 100644
>> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
>> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
>> @@ -16,6 +16,10 @@ Required Properties:
>> - "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1": rk3399 eMMC PHY
>> For this device it is strongly suggested to include arasan,soc-ctl-syscon.
>> - "ti,am654-sdhci-5.1", "arasan,sdhci-5.1": TI AM654 MMC PHY
>> + Note: This binding has been deprecated and moved to [5].
>> +
>> + [5] Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>> +
>> - reg: From mmc bindings: Register location and length.
>> - clocks: From clock bindings: Handles to clock inputs.
>> - clock-names: From clock bindings: Tuple including "clk_xin" and "clk_ahb"
>>
>
> I would prefer if you can extend commit message by description you have
> in cover letter which explains reasons. When this is applied and someone
> look at git history none will understand from first read why you have
> done that.
>
Ok. Will add full explanations to both documentation patches.
Thanks,
Faiz
On 29/11/18 6:15 PM, Faiz Abbas wrote:
> The host controllers on TI's AM654 SOCs are not compatible with
> the phy and consumer model of the sdhci-of-arasan driver. It turns out
> that for optimal operation at higher speeds, a special tuning procedure
> needs to be implemented which involves configuration of platform
> specific phy registers.
>
> Therefore, branch out to a new sdhci_am654 driver and add the phy
> register space with all phy configurations to it. Populate AM654
> specific callbacks to sdhci_ops and add SDHCI_QUIRKS wherever
> applicable.
>
> Only add support for upto High Speed for SD card and upto DDR52 speed
> mode for eMMC. Higher speeds will be added in subsequent patches.
>
> Signed-off-by: Faiz Abbas <[email protected]>
> ---
> drivers/mmc/host/Kconfig | 12 +
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/sdhci-of-arasan.c | 46 ----
> drivers/mmc/host/sdhci_am654.c | 376 +++++++++++++++++++++++++++++
> 4 files changed, 389 insertions(+), 46 deletions(-)
> create mode 100644 drivers/mmc/host/sdhci_am654.c
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 1b58739d9744..cfb2eb1a2c32 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -977,3 +977,15 @@ config MMC_SDHCI_OMAP
> If you have a controller with this interface, say Y or M here.
>
> If unsure, say N.
> +
> +config MMC_SDHCI_AM654
> + tristate "Support for the SDHCI Controller in TI's AM654 SOCs"
> + depends on MMC_SDHCI_PLTFM && OF
> + help
> + This selects the Secure Digital Host Controller Interface (SDHCI)
> + support present in TI's AM654 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 720d37777098..5c7770edc431 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -22,6 +22,7 @@ obj-$(CONFIG_MMC_SDHCI_S3C) += sdhci-s3c.o
> obj-$(CONFIG_MMC_SDHCI_SIRF) += sdhci-sirf.o
> obj-$(CONFIG_MMC_SDHCI_F_SDH30) += sdhci_f_sdh30.o
> obj-$(CONFIG_MMC_SDHCI_SPEAR) += sdhci-spear.o
> +obj-$(CONFIG_MMC_SDHCI_AM654) += sdhci_am654.o
> obj-$(CONFIG_MMC_WBSD) += wbsd.o
> obj-$(CONFIG_MMC_AU1X) += au1xmmc.o
> obj-$(CONFIG_MMC_MTK) += mtk-sd.o
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index 142c4b802f31..c9e3e050ccc8 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -231,25 +231,6 @@ static void sdhci_arasan_set_clock(struct sdhci_host *host, unsigned int clock)
> }
> }
>
> -static void sdhci_arasan_am654_set_clock(struct sdhci_host *host,
> - unsigned int clock)
> -{
> - struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> - struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
> -
> - if (sdhci_arasan->is_phy_on) {
> - phy_power_off(sdhci_arasan->phy);
> - sdhci_arasan->is_phy_on = false;
> - }
> -
> - sdhci_set_clock(host, clock);
> -
> - if (clock > PHY_CLK_TOO_SLOW_HZ) {
> - phy_power_on(sdhci_arasan->phy);
> - sdhci_arasan->is_phy_on = true;
> - }
> -}
> -
> static void sdhci_arasan_hs400_enhanced_strobe(struct mmc_host *mmc,
> struct mmc_ios *ios)
> {
> @@ -335,29 +316,6 @@ static struct sdhci_arasan_of_data sdhci_arasan_data = {
> .pdata = &sdhci_arasan_pdata,
> };
>
> -static const struct sdhci_ops sdhci_arasan_am654_ops = {
> - .set_clock = sdhci_arasan_am654_set_clock,
> - .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> - .get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
> - .set_bus_width = sdhci_set_bus_width,
> - .reset = sdhci_arasan_reset,
> - .set_uhs_signaling = sdhci_set_uhs_signaling,
> -};
> -
> -static const struct sdhci_pltfm_data sdhci_arasan_am654_pdata = {
> - .ops = &sdhci_arasan_am654_ops,
> - .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> - SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
> - SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
> - .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
> - SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN |
> - SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400,
> -};
> -
> -static const struct sdhci_arasan_of_data sdhci_arasan_am654_data = {
> - .pdata = &sdhci_arasan_am654_pdata,
> -};
> -
> static u32 sdhci_arasan_cqhci_irq(struct sdhci_host *host, u32 intmask)
> {
> int cmd_error = 0;
> @@ -520,10 +478,6 @@ static const struct of_device_id sdhci_arasan_of_match[] = {
> .compatible = "rockchip,rk3399-sdhci-5.1",
> .data = &sdhci_arasan_rk3399_data,
> },
> - {
> - .compatible = "ti,am654-sdhci-5.1",
> - .data = &sdhci_arasan_am654_data,
> - },
> /* Generic compatible below here */
> {
> .compatible = "arasan,sdhci-8.9a",
> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
> new file mode 100644
> index 000000000000..6e5be12d5946
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci_am654.c
> @@ -0,0 +1,376 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * sdhci_am654.c - SDHCI driver for TI's AM654 SOCs
> + *
> + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com
> + *
> + */
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +
> +#include "sdhci-pltfm.h"
> +
> +/* CTL_CFG Registers */
> +#define CTL_CFG_2 0x14
> +
> +#define SLOTTYPE_MASK GENMASK(31, 30)
> +#define SLOTTYPE_EMBEDDED BIT(30)
> +
> +/* PHY Registers */
> +#define PHY_CTRL1 0x100
> +#define PHY_CTRL2 0x104
> +#define PHY_CTRL3 0x108
> +#define PHY_CTRL4 0x10C
> +#define PHY_CTRL5 0x110
> +#define PHY_CTRL6 0x114
> +#define PHY_STAT1 0x130
> +#define PHY_STAT2 0x134
> +
> +#define IOMUX_ENABLE_SHIFT 31
> +#define IOMUX_ENABLE_MASK BIT(IOMUX_ENABLE_SHIFT)
> +#define OTAPDLYENA_SHIFT 20
> +#define OTAPDLYENA_MASK BIT(OTAPDLYENA_SHIFT)
> +#define OTAPDLYSEL_SHIFT 12
> +#define OTAPDLYSEL_MASK GENMASK(15, 12)
> +#define STRBSEL_SHIFT 24
> +#define STRBSEL_MASK GENMASK(27, 24)
> +#define SEL50_SHIFT 8
> +#define SEL50_MASK BIT(SEL50_SHIFT)
> +#define SEL100_SHIFT 9
> +#define SEL100_MASK BIT(SEL100_SHIFT)
> +#define DLL_TRIM_ICP_SHIFT 4
> +#define DLL_TRIM_ICP_MASK GENMASK(7, 4)
> +#define DR_TY_SHIFT 20
> +#define DR_TY_MASK GENMASK(22, 20)
> +#define ENDLL_SHIFT 1
> +#define ENDLL_MASK BIT(ENDLL_SHIFT)
> +#define DLLRDY_SHIFT 0
> +#define DLLRDY_MASK BIT(DLLRDY_SHIFT)
> +#define PDB_SHIFT 0
> +#define PDB_MASK BIT(PDB_SHIFT)
> +#define CALDONE_SHIFT 1
> +#define CALDONE_MASK BIT(CALDONE_SHIFT)
> +#define RETRIM_SHIFT 17
> +#define RETRIM_MASK BIT(RETRIM_SHIFT)
> +
> +#define DRIVER_STRENGTH_50_OHM 0x0
> +#define DRIVER_STRENGTH_33_OHM 0x1
> +#define DRIVER_STRENGTH_66_OHM 0x2
> +#define DRIVER_STRENGTH_100_OHM 0x3
> +#define DRIVER_STRENGTH_40_OHM 0x4
> +
> +#define CLOCK_TOO_SLOW_HZ 400000
> +
> +static struct regmap_config sdhci_am654_regmap_config = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .fast_io = true,
> +};
> +
> +struct sdhci_am654_data {
> + struct regmap *base;
> + struct clk *clk_ahb;
> + int otap_del_sel;
> + int trm_icp;
> + int drv_strength;
> + bool dll_on;
> +};
> +
> +static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
> + int sel50, sel100;
> + u32 mask, val;
> + int ret;
> +
> + if (sdhci_am654->dll_on) {
> + regmap_update_bits(sdhci_am654->base, PHY_CTRL1,
> + ENDLL_MASK, 0);
> +
> + sdhci_am654->dll_on = false;
> + }
> +
> + sdhci_set_clock(host, clock);
> +
> + if (clock > CLOCK_TOO_SLOW_HZ) {
> + /* Setup DLL Output TAP delay */
> + mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
> + val = (1 << OTAPDLYENA_SHIFT) |
> + (sdhci_am654->otap_del_sel << OTAPDLYSEL_SHIFT);
> + regmap_update_bits(sdhci_am654->base, PHY_CTRL4,
> + mask, val);
> + switch (clock) {
> + case 200000000:
> + sel50 = 0;
> + sel100 = 0;
> + break;
> + case 100000000:
> + sel50 = 0;
> + sel100 = 1;
> + break;
> + default:
> + sel50 = 1;
> + sel100 = 0;
> + }
> +
> + /* Configure PHY DLL frequency */
> + mask = SEL50_MASK | SEL100_MASK;
> + val = (sel50 << SEL50_SHIFT) | (sel100 << SEL100_SHIFT);
> + regmap_update_bits(sdhci_am654->base, PHY_CTRL5,
> + mask, val);
> + /* Configure DLL TRIM */
> + mask = DLL_TRIM_ICP_MASK;
> + val = sdhci_am654->trm_icp << DLL_TRIM_ICP_SHIFT;
> +
> + /* Configure DLL driver strength */
> + mask |= DR_TY_MASK;
> + val |= sdhci_am654->drv_strength << DR_TY_SHIFT;
> + regmap_update_bits(sdhci_am654->base, PHY_CTRL1,
> + mask, val);
> + /* Enable DLL */
> + regmap_update_bits(sdhci_am654->base, PHY_CTRL1,
> + ENDLL_MASK, 0x1 << ENDLL_SHIFT);
> + /*
> + * Poll for DLL ready. Use a one second timeout.
> + * Works in all experiments done so far
> + */
> + ret = regmap_read_poll_timeout(sdhci_am654->base,
> + PHY_STAT1, val,
> + val & DLLRDY_MASK,
> + 1000, 1000000);
> +
> + sdhci_am654->dll_on = true;
> + }
> +}
> +
> +
Double blank line
> +static void sdhci_am654_set_power(struct sdhci_host *host, unsigned char mode,
> + unsigned short vdd)
> +{
> + if (!IS_ERR(host->mmc->supply.vmmc)) {
> + struct mmc_host *mmc = host->mmc;
> +
> + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
> + }
> + sdhci_set_power_noreg(host, mode, vdd);
> +}
> +
> +struct sdhci_ops sdhci_am654_ops = {
> + .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> + .get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
> + .set_uhs_signaling = sdhci_set_uhs_signaling,
> + .set_bus_width = sdhci_set_bus_width,
> + .set_power = sdhci_am654_set_power,
> + .set_clock = sdhci_am654_set_clock,
> + .reset = sdhci_reset,
> +};
> +
> +static const struct sdhci_pltfm_data sdhci_am654_pdata = {
> + .ops = &sdhci_am654_ops,
> + .quirks = SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
> + SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
> + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> +};
> +
> +static int sdhci_am654_init(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
> + u32 ctl_cfg_2 = 0;
> + u32 val;
> + int ret;
> +
> + regmap_read(sdhci_am654->base, PHY_STAT1, &val);
> + if (~val & CALDONE_MASK) {
> + /* Calibrate IO lines */
> + regmap_update_bits(sdhci_am654->base, PHY_CTRL1,
> + PDB_MASK, PDB_MASK);
> + ret = regmap_read_poll_timeout(sdhci_am654->base, PHY_STAT1,
> + val, val & CALDONE_MASK, 1, 20);
> + if (ret)
> + return ret;
> + }
> +
> + /* Enable pins by setting IO mux to 0 */
> + regmap_update_bits(sdhci_am654->base, PHY_CTRL1,
> + IOMUX_ENABLE_MASK, 0);
> +
> + /* Set slot type based on SD or eMMC */
> + if (host->mmc->caps & MMC_CAP_NONREMOVABLE)
> + ctl_cfg_2 = SLOTTYPE_EMBEDDED;
> +
> + regmap_update_bits(sdhci_am654->base, CTL_CFG_2,
> + ctl_cfg_2, SLOTTYPE_MASK);
> +
> + return sdhci_add_host(host);
> +}
> +
> +static int sdhci_am654_get_of_property(struct platform_device *pdev,
> + struct sdhci_am654_data *sdhci_am654)
> +{
> + struct device *dev = &pdev->dev;
> + int drv_strength;
> + int ret;
> +
> + ret = device_property_read_u32(dev, "ti,trm-icp",
> + &sdhci_am654->trm_icp);
> + if (ret)
> + return ret;
> +
> + ret = device_property_read_u32(dev, "ti,otap-del-sel",
> + &sdhci_am654->otap_del_sel);
> + if (ret)
> + return ret;
> +
> + ret = device_property_read_u32(dev, "ti,driver-strength-ohm",
> + &drv_strength);
> + if (ret)
> + return ret;
> +
> + switch (drv_strength) {
> + case 50:
> + sdhci_am654->drv_strength = DRIVER_STRENGTH_50_OHM;
> + break;
> + case 33:
> + sdhci_am654->drv_strength = DRIVER_STRENGTH_33_OHM;
> + break;
> + case 66:
> + sdhci_am654->drv_strength = DRIVER_STRENGTH_66_OHM;
> + break;
> + case 100:
> + sdhci_am654->drv_strength = DRIVER_STRENGTH_100_OHM;
> + break;
> + case 40:
> + sdhci_am654->drv_strength = DRIVER_STRENGTH_40_OHM;
> + break;
> + default:
> + dev_err(dev, "Invalid driver strength\n");
> + return -EINVAL;
> + }
> +
> + sdhci_get_of_property(pdev);
> +
> + return 0;
> +}
> +
> +static int sdhci_am654_probe(struct platform_device *pdev)
> +{
> + struct sdhci_pltfm_host *pltfm_host;
> + struct sdhci_am654_data *sdhci_am654;
> + struct sdhci_host *host;
> + struct resource *res;
> + struct clk *clk_xin;
> + struct device *dev = &pdev->dev;
> + void __iomem *base;
> + int ret;
> +
> + host = sdhci_pltfm_init(pdev, &sdhci_am654_pdata, sizeof(*sdhci_am654));
> + if (IS_ERR(host))
> + return PTR_ERR(host);
> +
> + pltfm_host = sdhci_priv(host);
> + sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
> +
> + clk_xin = devm_clk_get(dev, "clk_xin");
> + if (IS_ERR(clk_xin)) {
> + dev_err(dev, "clk_xin clock not found.\n");
> + ret = PTR_ERR(clk_xin);
> + goto err_pltfm_free;
> + }
> +
> + sdhci_am654->clk_ahb = devm_clk_get(dev, "clk_ahb");
> + if (IS_ERR(sdhci_am654->clk_ahb)) {
> + dev_err(dev, "clk_ahb clock not found.\n");
> + ret = PTR_ERR(sdhci_am654->clk_ahb);
> + goto err_pltfm_free;
> + }
Did you intend not to enable clks?
> +
> + pltfm_host->clk = clk_xin;
> +
> + pm_runtime_enable(dev);
> + ret = pm_runtime_get_sync(dev);
> + if (ret > 0) {
Did you intend 'ret > 0'?
> + pm_runtime_put_noidle(dev);
> + goto pm_runtime_disable;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(base)) {
> + ret = PTR_ERR(base);
> + goto pm_runtime_put;
> + }
> +
> + sdhci_am654->base = devm_regmap_init_mmio(dev, base,
> + &sdhci_am654_regmap_config);
> + if (IS_ERR(sdhci_am654->base)) {
> + dev_err(dev, "Failed to initialize regmap\n");
> + ret = PTR_ERR(sdhci_am654->base);
> + goto pm_runtime_put;
> + }
> +
> + ret = sdhci_am654_get_of_property(pdev, sdhci_am654);
> + if (ret)
> + goto pm_runtime_put;
> +
> + ret = mmc_of_parse(host->mmc);
> + if (ret) {
> + dev_err(dev, "parsing dt failed (%d)\n", ret);
> + goto pm_runtime_put;
> + }
> +
> + ret = sdhci_am654_init(host);
> + if (ret)
> + goto pm_runtime_put;
> +
> + return 0;
> +
> +pm_runtime_put:
> + pm_runtime_put_sync(dev);
> +pm_runtime_disable:
> + pm_runtime_disable(dev);
> +err_pltfm_free:
> + sdhci_pltfm_free(pdev);
> + return ret;
> +}
> +
> +static int sdhci_am654_remove(struct platform_device *pdev)
> +{
> + struct sdhci_host *host = platform_get_drvdata(pdev);
> + int ret;
> +
> + sdhci_remove_host(host, true);
> + ret = pm_runtime_put_sync(&pdev->dev);
> + if (ret < 0)
> + return ret;
> +
> + pm_runtime_disable(&pdev->dev);
> + sdhci_pltfm_free(pdev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sdhci_am654_of_match[] = {
> + { .compatible = "ti,am654-sdhci-5.1" },
> + { /* sentinel */ }
> +};
> +
> +static struct platform_driver sdhci_am654_driver = {
> + .driver = {
> + .name = "sdhci-am654",
> + .of_match_table = sdhci_am654_of_match,
> + },
> + .probe = sdhci_am654_probe,
> + .remove = sdhci_am654_remove,
> +};
> +
> +module_platform_driver(sdhci_am654_driver);
> +
> +MODULE_DESCRIPTION("Driver for SDHCI Controller on TI's AM654 devices");
> +MODULE_AUTHOR("Faiz Abbas <[email protected]>");
> +MODULE_LICENSE("GPL");
>
Hi Adrian,
On 05/12/18 7:12 PM, Adrian Hunter wrote:
> On 29/11/18 6:15 PM, Faiz Abbas wrote:
>> The host controllers on TI's AM654 SOCs are not compatible with
>> the phy and consumer model of the sdhci-of-arasan driver. It turns out
>> that for optimal operation at higher speeds, a special tuning procedure
>> needs to be implemented which involves configuration of platform
>> specific phy registers.
>>
>> Therefore, branch out to a new sdhci_am654 driver and add the phy
>> register space with all phy configurations to it. Populate AM654
>> specific callbacks to sdhci_ops and add SDHCI_QUIRKS wherever
>> applicable.
>>
>> Only add support for upto High Speed for SD card and upto DDR52 speed
>> mode for eMMC. Higher speeds will be added in subsequent patches.
>>
...
>> + sdhci_am654->dll_on = true;
>> + }
>> +}
>> +
>> +
>
> Double blank line
Will fix.
>
>> +static void sdhci_am654_set_power(struct sdhci_host *host, unsigned char mode,
>> + unsigned short vdd)
>> +{
>> + if (!IS_ERR(host->mmc->supply.vmmc)) {
>> + struct mmc_host *mmc = host->mmc;
>> +
>> + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
>> + }
>> + sdhci_set_power_noreg(host, mode, vdd);
>> +}
>> +
>> +struct sdhci_ops sdhci_am654_ops = {
>> + .get_max_clock = sdhci_pltfm_clk_get_max_clock,
>> + .get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
>> + .set_uhs_signaling = sdhci_set_uhs_signaling,
>> + .set_bus_width = sdhci_set_bus_width,
>> + .set_power = sdhci_am654_set_power,
>> + .set_clock = sdhci_am654_set_clock,
>> + .reset = sdhci_reset,
>> +};
>> +
>> +static const struct sdhci_pltfm_data sdhci_am654_pdata = {
>> + .ops = &sdhci_am654_ops,
>> + .quirks = SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
>> + SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
>> + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
>> +};
>> +
>> +static int sdhci_am654_init(struct sdhci_host *host)
>> +{
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
>> + u32 ctl_cfg_2 = 0;
>> + u32 val;
>> + int ret;
>> +
>> + regmap_read(sdhci_am654->base, PHY_STAT1, &val);
>> + if (~val & CALDONE_MASK) {
>> + /* Calibrate IO lines */
>> + regmap_update_bits(sdhci_am654->base, PHY_CTRL1,
>> + PDB_MASK, PDB_MASK);
>> + ret = regmap_read_poll_timeout(sdhci_am654->base, PHY_STAT1,
>> + val, val & CALDONE_MASK, 1, 20);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + /* Enable pins by setting IO mux to 0 */
>> + regmap_update_bits(sdhci_am654->base, PHY_CTRL1,
>> + IOMUX_ENABLE_MASK, 0);
>> +
>> + /* Set slot type based on SD or eMMC */
>> + if (host->mmc->caps & MMC_CAP_NONREMOVABLE)
>> + ctl_cfg_2 = SLOTTYPE_EMBEDDED;
>> +
>> + regmap_update_bits(sdhci_am654->base, CTL_CFG_2,
>> + ctl_cfg_2, SLOTTYPE_MASK);
>> +
>> + return sdhci_add_host(host);
>> +}
>> +
>> +static int sdhci_am654_get_of_property(struct platform_device *pdev,
>> + struct sdhci_am654_data *sdhci_am654)
>> +{
>> + struct device *dev = &pdev->dev;
>> + int drv_strength;
>> + int ret;
>> +
>> + ret = device_property_read_u32(dev, "ti,trm-icp",
>> + &sdhci_am654->trm_icp);
>> + if (ret)
>> + return ret;
>> +
>> + ret = device_property_read_u32(dev, "ti,otap-del-sel",
>> + &sdhci_am654->otap_del_sel);
>> + if (ret)
>> + return ret;
>> +
>> + ret = device_property_read_u32(dev, "ti,driver-strength-ohm",
>> + &drv_strength);
>> + if (ret)
>> + return ret;
>> +
>> + switch (drv_strength) {
>> + case 50:
>> + sdhci_am654->drv_strength = DRIVER_STRENGTH_50_OHM;
>> + break;
>> + case 33:
>> + sdhci_am654->drv_strength = DRIVER_STRENGTH_33_OHM;
>> + break;
>> + case 66:
>> + sdhci_am654->drv_strength = DRIVER_STRENGTH_66_OHM;
>> + break;
>> + case 100:
>> + sdhci_am654->drv_strength = DRIVER_STRENGTH_100_OHM;
>> + break;
>> + case 40:
>> + sdhci_am654->drv_strength = DRIVER_STRENGTH_40_OHM;
>> + break;
>> + default:
>> + dev_err(dev, "Invalid driver strength\n");
>> + return -EINVAL;
>> + }
>> +
>> + sdhci_get_of_property(pdev);
>> +
>> + return 0;
>> +}
>> +
>> +static int sdhci_am654_probe(struct platform_device *pdev)
>> +{
>> + struct sdhci_pltfm_host *pltfm_host;
>> + struct sdhci_am654_data *sdhci_am654;
>> + struct sdhci_host *host;
>> + struct resource *res;
>> + struct clk *clk_xin;
>> + struct device *dev = &pdev->dev;
>> + void __iomem *base;
>> + int ret;
>> +
>> + host = sdhci_pltfm_init(pdev, &sdhci_am654_pdata, sizeof(*sdhci_am654));
>> + if (IS_ERR(host))
>> + return PTR_ERR(host);
>> +
>> + pltfm_host = sdhci_priv(host);
>> + sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
>> +
>> + clk_xin = devm_clk_get(dev, "clk_xin");
>> + if (IS_ERR(clk_xin)) {
>> + dev_err(dev, "clk_xin clock not found.\n");
>> + ret = PTR_ERR(clk_xin);
>> + goto err_pltfm_free;
>> + }
>> +
>> + sdhci_am654->clk_ahb = devm_clk_get(dev, "clk_ahb");
>> + if (IS_ERR(sdhci_am654->clk_ahb)) {
>> + dev_err(dev, "clk_ahb clock not found.\n");
>> + ret = PTR_ERR(sdhci_am654->clk_ahb);
>> + goto err_pltfm_free;
>> + }
>
> Did you intend not to enable clks?
Yes. Clocks get enabled as a part of pm_runtime calls.
>
>> +
>> + pltfm_host->clk = clk_xin;
>> +
>> + pm_runtime_enable(dev);
>> + ret = pm_runtime_get_sync(dev);
>> + if (ret > 0) {
>
> Did you intend 'ret > 0'?
Sorry. That was intended to be < 0.
Thanks,
Faiz
On 5/12/18 5:07 PM, Faiz Abbas wrote:
> Hi Adrian,
>
> On 05/12/18 7:12 PM, Adrian Hunter wrote:
>> On 29/11/18 6:15 PM, Faiz Abbas wrote:
>>> The host controllers on TI's AM654 SOCs are not compatible with
>>> the phy and consumer model of the sdhci-of-arasan driver. It turns out
>>> that for optimal operation at higher speeds, a special tuning procedure
>>> needs to be implemented which involves configuration of platform
>>> specific phy registers.
>>>
>>> Therefore, branch out to a new sdhci_am654 driver and add the phy
>>> register space with all phy configurations to it. Populate AM654
>>> specific callbacks to sdhci_ops and add SDHCI_QUIRKS wherever
>>> applicable.
>>>
>>> Only add support for upto High Speed for SD card and upto DDR52 speed
>>> mode for eMMC. Higher speeds will be added in subsequent patches.
>>>
> ...
>>> + sdhci_am654->dll_on = true;
>>> + }
>>> +}
>>> +
>>> +
>>
>> Double blank line
>
> Will fix.
>
>>
>>> +static void sdhci_am654_set_power(struct sdhci_host *host, unsigned char mode,
>>> + unsigned short vdd)
>>> +{
>>> + if (!IS_ERR(host->mmc->supply.vmmc)) {
>>> + struct mmc_host *mmc = host->mmc;
>>> +
>>> + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
>>> + }
>>> + sdhci_set_power_noreg(host, mode, vdd);
>>> +}
>>> +
>>> +struct sdhci_ops sdhci_am654_ops = {
>>> + .get_max_clock = sdhci_pltfm_clk_get_max_clock,
>>> + .get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
>>> + .set_uhs_signaling = sdhci_set_uhs_signaling,
>>> + .set_bus_width = sdhci_set_bus_width,
>>> + .set_power = sdhci_am654_set_power,
>>> + .set_clock = sdhci_am654_set_clock,
>>> + .reset = sdhci_reset,
>>> +};
>>> +
>>> +static const struct sdhci_pltfm_data sdhci_am654_pdata = {
>>> + .ops = &sdhci_am654_ops,
>>> + .quirks = SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
>>> + SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
>>> + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
>>> +};
>>> +
>>> +static int sdhci_am654_init(struct sdhci_host *host)
>>> +{
>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> + struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
>>> + u32 ctl_cfg_2 = 0;
>>> + u32 val;
>>> + int ret;
>>> +
>>> + regmap_read(sdhci_am654->base, PHY_STAT1, &val);
>>> + if (~val & CALDONE_MASK) {
>>> + /* Calibrate IO lines */
>>> + regmap_update_bits(sdhci_am654->base, PHY_CTRL1,
>>> + PDB_MASK, PDB_MASK);
>>> + ret = regmap_read_poll_timeout(sdhci_am654->base, PHY_STAT1,
>>> + val, val & CALDONE_MASK, 1, 20);
>>> + if (ret)
>>> + return ret;
>>> + }
>>> +
>>> + /* Enable pins by setting IO mux to 0 */
>>> + regmap_update_bits(sdhci_am654->base, PHY_CTRL1,
>>> + IOMUX_ENABLE_MASK, 0);
>>> +
>>> + /* Set slot type based on SD or eMMC */
>>> + if (host->mmc->caps & MMC_CAP_NONREMOVABLE)
>>> + ctl_cfg_2 = SLOTTYPE_EMBEDDED;
>>> +
>>> + regmap_update_bits(sdhci_am654->base, CTL_CFG_2,
>>> + ctl_cfg_2, SLOTTYPE_MASK);
>>> +
>>> + return sdhci_add_host(host);
>>> +}
>>> +
>>> +static int sdhci_am654_get_of_property(struct platform_device *pdev,
>>> + struct sdhci_am654_data *sdhci_am654)
>>> +{
>>> + struct device *dev = &pdev->dev;
>>> + int drv_strength;
>>> + int ret;
>>> +
>>> + ret = device_property_read_u32(dev, "ti,trm-icp",
>>> + &sdhci_am654->trm_icp);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = device_property_read_u32(dev, "ti,otap-del-sel",
>>> + &sdhci_am654->otap_del_sel);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = device_property_read_u32(dev, "ti,driver-strength-ohm",
>>> + &drv_strength);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + switch (drv_strength) {
>>> + case 50:
>>> + sdhci_am654->drv_strength = DRIVER_STRENGTH_50_OHM;
>>> + break;
>>> + case 33:
>>> + sdhci_am654->drv_strength = DRIVER_STRENGTH_33_OHM;
>>> + break;
>>> + case 66:
>>> + sdhci_am654->drv_strength = DRIVER_STRENGTH_66_OHM;
>>> + break;
>>> + case 100:
>>> + sdhci_am654->drv_strength = DRIVER_STRENGTH_100_OHM;
>>> + break;
>>> + case 40:
>>> + sdhci_am654->drv_strength = DRIVER_STRENGTH_40_OHM;
>>> + break;
>>> + default:
>>> + dev_err(dev, "Invalid driver strength\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + sdhci_get_of_property(pdev);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int sdhci_am654_probe(struct platform_device *pdev)
>>> +{
>>> + struct sdhci_pltfm_host *pltfm_host;
>>> + struct sdhci_am654_data *sdhci_am654;
>>> + struct sdhci_host *host;
>>> + struct resource *res;
>>> + struct clk *clk_xin;
>>> + struct device *dev = &pdev->dev;
>>> + void __iomem *base;
>>> + int ret;
>>> +
>>> + host = sdhci_pltfm_init(pdev, &sdhci_am654_pdata, sizeof(*sdhci_am654));
>>> + if (IS_ERR(host))
>>> + return PTR_ERR(host);
>>> +
>>> + pltfm_host = sdhci_priv(host);
>>> + sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
>>> +
>>> + clk_xin = devm_clk_get(dev, "clk_xin");
>>> + if (IS_ERR(clk_xin)) {
>>> + dev_err(dev, "clk_xin clock not found.\n");
>>> + ret = PTR_ERR(clk_xin);
>>> + goto err_pltfm_free;
>>> + }
>>> +
>>> + sdhci_am654->clk_ahb = devm_clk_get(dev, "clk_ahb");
>>> + if (IS_ERR(sdhci_am654->clk_ahb)) {
>>> + dev_err(dev, "clk_ahb clock not found.\n");
>>> + ret = PTR_ERR(sdhci_am654->clk_ahb);
>>> + goto err_pltfm_free;
>>> + }
>>
>> Did you intend not to enable clks?
>
> Yes. Clocks get enabled as a part of pm_runtime calls.
Ok, but that could use an explanatory comment. Also why get a reference to
clk_ahb if that reference is never used?
>>
>>> +
>>> + pltfm_host->clk = clk_xin;
>>> +
>>> + pm_runtime_enable(dev);
>>> + ret = pm_runtime_get_sync(dev);
>>> + if (ret > 0) {
>>
>> Did you intend 'ret > 0'?
>
> Sorry. That was intended to be < 0.
>
> Thanks,
> Faiz
>
Hi Adrian,
On 07/12/18 7:02 PM, Adrian Hunter wrote:
> On 5/12/18 5:07 PM, Faiz Abbas wrote:
>> Hi Adrian,
>>
>> On 05/12/18 7:12 PM, Adrian Hunter wrote:
>>> On 29/11/18 6:15 PM, Faiz Abbas wrote:
>>>> The host controllers on TI's AM654 SOCs are not compatible with
>>>> the phy and consumer model of the sdhci-of-arasan driver. It turns out
>>>> that for optimal operation at higher speeds, a special tuning procedure
>>>> needs to be implemented which involves configuration of platform
>>>> specific phy registers.
>>>>
>>>> Therefore, branch out to a new sdhci_am654 driver and add the phy
>>>> register space with all phy configurations to it. Populate AM654
>>>> specific callbacks to sdhci_ops and add SDHCI_QUIRKS wherever
>>>> applicable.
>>>>
>>>> Only add support for upto High Speed for SD card and upto DDR52 speed
>>>> mode for eMMC. Higher speeds will be added in subsequent patches.
>>>>
...
>>>> +
>>>> + sdhci_am654->clk_ahb = devm_clk_get(dev, "clk_ahb");
>>>> + if (IS_ERR(sdhci_am654->clk_ahb)) {
>>>> + dev_err(dev, "clk_ahb clock not found.\n");
>>>> + ret = PTR_ERR(sdhci_am654->clk_ahb);
>>>> + goto err_pltfm_free;
>>>> + }
>>>
>>> Did you intend not to enable clks?
>>
>> Yes. Clocks get enabled as a part of pm_runtime calls.
>
> Ok, but that could use an explanatory comment. Also why get a reference to
> clk_ahb if that reference is never used?
>
You're right. It was being used in sdhci-of-arasan because other users
needed to call enable() and disable(). I missed out on removing it when
porting over. Will remove it and add the comment.
Thanks,
Faiz