From: Adrian Hunter <adrian.hunter@intel.com> To: Kishon Vijay Abraham I <kishon@ti.com>, Ulf Hansson <ulf.hansson@linaro.org> Cc: Rob Herring <robh+dt@kernel.org>, Tony Lindgren <tony@atomide.com>, Sekhar Nori <nsekhar@ti.com>, Russell King <linux@armlinux.org.uk>, Ravikumar Kattekola <rk@ti.com>, linux-mmc@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 4/5] mmc: sdhci-omap: Add OMAP SDHCI driver Date: Mon, 28 Aug 2017 12:06:38 +0300 [thread overview] Message-ID: <4f0a7225-1843-b353-1a2a-9adcb49554d3@intel.com> (raw) In-Reply-To: <20170821074132.4622-5-kishon@ti.com> On 21/08/17 10:41, 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. > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> There are a few minor comments below. > --- > drivers/mmc/host/Kconfig | 12 + > drivers/mmc/host/Makefile | 1 + > drivers/mmc/host/sdhci-omap.c | 629 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 642 insertions(+) > create mode 100644 drivers/mmc/host/sdhci-omap.c > > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig > index 60f90d49e7a9..60d50843e165 100644 > --- a/drivers/mmc/host/Kconfig > +++ b/drivers/mmc/host/Kconfig > @@ -899,3 +899,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 8c46766c000c..33181df88a10 100644 > --- a/drivers/mmc/host/Makefile > +++ b/drivers/mmc/host/Makefile > @@ -88,6 +88,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..73ee3b4a400c > --- /dev/null > +++ b/drivers/mmc/host/sdhci-omap.c > @@ -0,0 +1,629 @@ > +/** > + * SDHCI Controller driver for TI's OMAP SoCs > + * > + * Copyright (C) 2017 Texas Instruments > + * Author: 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 of > + * the License as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#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 */ As before, SDHCI_OMAP_TIMEOUT is a better name > + > +#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 bus_mode; > + 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_bus_mode(struct sdhci_omap_host *omap_host, > + unsigned int mode) > +{ > + u32 reg; > + > + if (omap_host->bus_mode == mode) > + return; > + > + 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); > + > + omap_host->bus_mode = mode; > +} > + > +void sdhci_omap_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) > +{ > + struct sdhci_host *host = mmc_priv(mmc); > + struct sdhci_pltfm_host *pltfm_host; > + struct sdhci_omap_host *omap_host; > + > + pltfm_host = sdhci_priv(host); > + omap_host = sdhci_pltfm_priv(pltfm_host); > + > + sdhci_omap_set_bus_mode(omap_host, ios->bus_mode); > + sdhci_set_ios(mmc, ios); > +} > + > +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 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; Are you sure you don't want to stop the clock. > + > + 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); > +} > + > +void sdhci_omap_set_power(struct sdhci_host *host, unsigned char mode, > + unsigned short vdd) > +{ > + struct mmc_host *mmc = host->mmc; > + > + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd); > +} > + > +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 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 struct sdhci_ops sdhci_omap_ops = { > + .set_clock = sdhci_omap_set_clock, > + .set_power = sdhci_omap_set_power, > + .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, > +}; > + > +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_RSP_136_HAS_CRC, > + .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; > + > + /* Enable main Func clock, interface clock and program SYSCONFIG */ Maybe also mention that the PM domain does those things. Out of curiousity, what happens if CONFIG_PM is not set? > + 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; The correct "undo" function for pm_runtime_get_sync() failing is pm_runtime_put_noidle() not pm_runtime_put_sync(). So: pm_runtime_put_noidle(dev); goto err_rpm_disable; > + } > + > + 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; > + host->mmc_host_ops.set_ios = sdhci_omap_set_ios; > + > + sdhci_read_caps(host); > + host->caps |= SDHCI_CAN_DO_ADMA2; > + > + ret = sdhci_add_host(host); > + if (ret) > + goto err_get_sync; > + > + return 0; > + > +err_get_sync: It is more usual for error labels to describe what they will do rather than what failed. i.e. this would be err_put_sync: > + pm_runtime_put_sync(dev); err_rpm_disable: > + pm_runtime_disable(dev); > + > +err_of_parse: err_pltfm_free: > + 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), Don't you need #ifdef CONFIG_OF around the table if you use of_match_ptr()? > + }, > +}; > + > +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"); A module alias with spaces in it looks strange. Is that the intention?
WARNING: multiple messages have this Message-ID (diff)
From: adrian.hunter@intel.com (Adrian Hunter) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 4/5] mmc: sdhci-omap: Add OMAP SDHCI driver Date: Mon, 28 Aug 2017 12:06:38 +0300 [thread overview] Message-ID: <4f0a7225-1843-b353-1a2a-9adcb49554d3@intel.com> (raw) In-Reply-To: <20170821074132.4622-5-kishon@ti.com> On 21/08/17 10:41, 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. > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> There are a few minor comments below. > --- > drivers/mmc/host/Kconfig | 12 + > drivers/mmc/host/Makefile | 1 + > drivers/mmc/host/sdhci-omap.c | 629 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 642 insertions(+) > create mode 100644 drivers/mmc/host/sdhci-omap.c > > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig > index 60f90d49e7a9..60d50843e165 100644 > --- a/drivers/mmc/host/Kconfig > +++ b/drivers/mmc/host/Kconfig > @@ -899,3 +899,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 8c46766c000c..33181df88a10 100644 > --- a/drivers/mmc/host/Makefile > +++ b/drivers/mmc/host/Makefile > @@ -88,6 +88,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..73ee3b4a400c > --- /dev/null > +++ b/drivers/mmc/host/sdhci-omap.c > @@ -0,0 +1,629 @@ > +/** > + * SDHCI Controller driver for TI's OMAP SoCs > + * > + * Copyright (C) 2017 Texas Instruments > + * Author: 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 of > + * the License as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#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 */ As before, SDHCI_OMAP_TIMEOUT is a better name > + > +#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 bus_mode; > + 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_bus_mode(struct sdhci_omap_host *omap_host, > + unsigned int mode) > +{ > + u32 reg; > + > + if (omap_host->bus_mode == mode) > + return; > + > + 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); > + > + omap_host->bus_mode = mode; > +} > + > +void sdhci_omap_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) > +{ > + struct sdhci_host *host = mmc_priv(mmc); > + struct sdhci_pltfm_host *pltfm_host; > + struct sdhci_omap_host *omap_host; > + > + pltfm_host = sdhci_priv(host); > + omap_host = sdhci_pltfm_priv(pltfm_host); > + > + sdhci_omap_set_bus_mode(omap_host, ios->bus_mode); > + sdhci_set_ios(mmc, ios); > +} > + > +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 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; Are you sure you don't want to stop the clock. > + > + 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); > +} > + > +void sdhci_omap_set_power(struct sdhci_host *host, unsigned char mode, > + unsigned short vdd) > +{ > + struct mmc_host *mmc = host->mmc; > + > + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd); > +} > + > +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 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 struct sdhci_ops sdhci_omap_ops = { > + .set_clock = sdhci_omap_set_clock, > + .set_power = sdhci_omap_set_power, > + .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, > +}; > + > +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_RSP_136_HAS_CRC, > + .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; > + > + /* Enable main Func clock, interface clock and program SYSCONFIG */ Maybe also mention that the PM domain does those things. Out of curiousity, what happens if CONFIG_PM is not set? > + 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; The correct "undo" function for pm_runtime_get_sync() failing is pm_runtime_put_noidle() not pm_runtime_put_sync(). So: pm_runtime_put_noidle(dev); goto err_rpm_disable; > + } > + > + 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; > + host->mmc_host_ops.set_ios = sdhci_omap_set_ios; > + > + sdhci_read_caps(host); > + host->caps |= SDHCI_CAN_DO_ADMA2; > + > + ret = sdhci_add_host(host); > + if (ret) > + goto err_get_sync; > + > + return 0; > + > +err_get_sync: It is more usual for error labels to describe what they will do rather than what failed. i.e. this would be err_put_sync: > + pm_runtime_put_sync(dev); err_rpm_disable: > + pm_runtime_disable(dev); > + > +err_of_parse: err_pltfm_free: > + 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), Don't you need #ifdef CONFIG_OF around the table if you use of_match_ptr()? > + }, > +}; > + > +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"); A module alias with spaces in it looks strange. Is that the intention?
next prev parent reply other threads:[~2017-08-28 9:13 UTC|newest] Thread overview: 86+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-08-21 7:41 [PATCH 0/5] mmc: Add OMAP SDHCI driver Kishon Vijay Abraham I 2017-08-21 7:41 ` Kishon Vijay Abraham I 2017-08-21 7:41 ` Kishon Vijay Abraham I 2017-08-21 7:41 ` [PATCH 1/5] mmc: sdhci: Tidy reading 136-bit responses Kishon Vijay Abraham I 2017-08-21 7:41 ` Kishon Vijay Abraham I 2017-08-21 7:41 ` Kishon Vijay Abraham I 2017-08-30 13:13 ` Ulf Hansson 2017-08-30 13:13 ` Ulf Hansson 2017-08-30 13:13 ` Ulf Hansson 2017-08-21 7:41 ` [PATCH 2/5] mmc: sdhci: Add quirk to indicate MMC_RSP_136 has CRC Kishon Vijay Abraham I 2017-08-21 7:41 ` Kishon Vijay Abraham I 2017-08-21 7:41 ` Kishon Vijay Abraham I 2017-08-28 7:57 ` Adrian Hunter 2017-08-28 7:57 ` Adrian Hunter 2017-08-30 13:13 ` Ulf Hansson 2017-08-30 13:13 ` Ulf Hansson 2017-08-30 13:13 ` Ulf Hansson 2017-08-21 7:41 ` [PATCH 3/5] dt-bindings: ti-omap-hsmmc: Document new compatible for sdhci omap Kishon Vijay Abraham I 2017-08-21 7:41 ` Kishon Vijay Abraham I 2017-08-21 7:41 ` Kishon Vijay Abraham I 2017-08-21 14:21 ` Tony Lindgren 2017-08-21 14:21 ` Tony Lindgren 2017-08-22 13:39 ` [PATCH v2 3/5] dt-bindings: sdhci-omap: Add bindings for the sdhci-omap controller Kishon Vijay Abraham I 2017-08-22 13:39 ` Kishon Vijay Abraham I 2017-08-22 13:39 ` Kishon Vijay Abraham I 2017-08-22 17:39 ` Tony Lindgren 2017-08-22 17:39 ` Tony Lindgren 2017-08-23 5:42 ` [PATCH v3 " Kishon Vijay Abraham I 2017-08-23 5:42 ` Kishon Vijay Abraham I 2017-08-23 5:42 ` Kishon Vijay Abraham I 2017-08-23 13:07 ` Ulf Hansson 2017-08-23 13:07 ` Ulf Hansson 2017-08-23 13:07 ` Ulf Hansson 2017-08-23 13:56 ` Kishon Vijay Abraham I 2017-08-23 13:56 ` Kishon Vijay Abraham I 2017-08-23 13:56 ` Kishon Vijay Abraham I 2017-08-24 11:29 ` Ulf Hansson 2017-08-24 11:29 ` Ulf Hansson 2017-08-24 11:29 ` Ulf Hansson 2017-08-29 11:20 ` Kishon Vijay Abraham I 2017-08-29 11:20 ` Kishon Vijay Abraham I 2017-08-29 11:20 ` Kishon Vijay Abraham I 2017-08-29 11:50 ` Sebastian Reichel 2017-08-29 11:50 ` Sebastian Reichel 2017-08-29 11:50 ` Sebastian Reichel 2017-08-29 13:58 ` Tony Lindgren 2017-08-29 13:58 ` Tony Lindgren 2017-08-29 13:58 ` Tony Lindgren 2017-08-29 14:43 ` Tony Lindgren 2017-08-29 14:43 ` Tony Lindgren 2017-08-29 14:43 ` Tony Lindgren 2017-08-29 17:09 ` Rob Herring 2017-08-29 17:09 ` Rob Herring 2017-08-29 17:09 ` Rob Herring 2017-08-29 17:39 ` Tony Lindgren 2017-08-29 17:39 ` Tony Lindgren 2017-08-29 17:39 ` Tony Lindgren 2017-09-05 8:52 ` Kishon Vijay Abraham I 2017-09-05 8:52 ` Kishon Vijay Abraham I 2017-09-05 8:52 ` Kishon Vijay Abraham I 2017-09-05 16:51 ` Tony Lindgren 2017-09-05 16:51 ` Tony Lindgren 2017-09-05 16:51 ` Tony Lindgren 2017-08-29 17:11 ` Rob Herring 2017-08-29 17:11 ` Rob Herring 2017-09-05 8:53 ` Kishon Vijay Abraham I 2017-09-05 8:53 ` Kishon Vijay Abraham I 2017-09-05 8:53 ` Kishon Vijay Abraham I 2017-08-21 7:41 ` [PATCH 4/5] mmc: sdhci-omap: Add OMAP SDHCI driver Kishon Vijay Abraham I 2017-08-21 7:41 ` Kishon Vijay Abraham I 2017-08-21 7:41 ` Kishon Vijay Abraham I 2017-08-28 9:06 ` Adrian Hunter [this message] 2017-08-28 9:06 ` Adrian Hunter 2017-08-30 13:53 ` Kishon Vijay Abraham I 2017-08-30 13:53 ` Kishon Vijay Abraham I 2017-08-30 13:53 ` Kishon Vijay Abraham I 2017-08-31 13:02 ` Adrian Hunter 2017-08-31 13:02 ` Adrian Hunter 2017-09-05 8:57 ` Kishon Vijay Abraham I 2017-09-05 8:57 ` Kishon Vijay Abraham I 2017-09-05 8:57 ` Kishon Vijay Abraham I 2017-08-21 7:41 ` [PATCH 5/5] MAINTAINERS: Add TI OMAP SDHCI Maintainer Kishon Vijay Abraham I 2017-08-21 7:41 ` Kishon Vijay Abraham I 2017-08-21 7:41 ` Kishon Vijay Abraham I 2017-08-28 9:07 ` Adrian Hunter 2017-08-28 9:07 ` Adrian Hunter
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=4f0a7225-1843-b353-1a2a-9adcb49554d3@intel.com \ --to=adrian.hunter@intel.com \ --cc=devicetree@vger.kernel.org \ --cc=kishon@ti.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mmc@vger.kernel.org \ --cc=linux-omap@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=nsekhar@ti.com \ --cc=rk@ti.com \ --cc=robh+dt@kernel.org \ --cc=tony@atomide.com \ --cc=ulf.hansson@linaro.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.