* [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
@ 2015-12-01 16:41 ` Carlo Caione
0 siblings, 0 replies; 22+ messages in thread
From: Carlo Caione @ 2015-12-01 16:41 UTC (permalink / raw)
To: linux-arm-kernel, linux-mmc, drake, jerry.cao, victor.wan,
linux-meson, ulf.hansson
Cc: Carlo Caione
From: Carlo Caione <carlo@endlessm.com>
Add a driver for the SD/MMC host found on the Amlogic MesonX SoCs. This
is an MMC controller which provides an interface between the application
processor and various memory cards. It supports the SD specification
v2.0 and the eMMC specification v4.41.
This patch adds also the binding documentation.
Signed-off-by: Carlo Caione <carlo@endlessm.com>
---
Changelog:
v2:
* fix typo in commit message
* avoid the bounce buffer (max_segs == 1)
v3:
* simplify meson_mmc_map_dma() and fix memory leak
v4:
* code and documentation cleanup
* removed redundant variables
* removed useless printing
* introduced MMC_CAP2_NO_SDIO
* removed state machine
* better spinlocks management and positioning
---
.../devicetree/bindings/mmc/meson-mmc.txt | 29 ++
drivers/mmc/host/Kconfig | 7 +
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/meson-mmc.c | 527 +++++++++++++++++++++
4 files changed, 564 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mmc/meson-mmc.txt
create mode 100644 drivers/mmc/host/meson-mmc.c
diff --git a/Documentation/devicetree/bindings/mmc/meson-mmc.txt b/Documentation/devicetree/bindings/mmc/meson-mmc.txt
new file mode 100644
index 0000000..7ff9e9e
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/meson-mmc.txt
@@ -0,0 +1,29 @@
+* Amlogic MesonX MMC controller
+
+The highspeed MMC host controller on Amlogic SoCs provides an interface
+for MMC, SD, SDIO and SDHC types of memory cards.
+
+Supported maximum speeds are the ones of the eMMC standard 4.41 as well
+as the speed of SD standard 2.0.
+
+Required properties:
+ - compatible : "amlogic,meson-mmc"
+ - reg : mmc controller base registers
+ - interrupts : mmc controller interrupt
+ - clocks : phandle to clock provider
+ - pinctrl-names : should contain "sdio_a" or "sdio_b"
+ - pinctrl-0: Should specify pin control groups used for this controller
+
+Optional properties:
+ - for cd, bus-width and additional generic mmc parameters
+ please refer to mmc.txt within this directory
+
+Examples:
+ mmc0: mmc@c1108c20 {
+ pinctrl-names = "sdio_b";
+ pinctrl-0 = <&mmc0_sd_b_pins>;
+ compatible = "amlogic,meson-mmc";
+ reg = <0xc1108c20 0x20>;
+ interrupts = <0 28 1>;
+ clocks = <&clkc CLKID_CLK81>;
+ };
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 1dee533..7f2c5ae 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -765,6 +765,13 @@ config MMC_REALTEK_USB
Say Y here to include driver code to support SD/MMC card interface
of Realtek RTS5129/39 series card reader
+config MMC_MESON
+ tristate "Amlogic MesonX SD/MMC Host Controller support"
+ depends on ARCH_MESON
+ help
+ This selects support for the SD/MMC Host Controller on
+ Amlogic MesonX SoCs.
+
config MMC_SUNXI
tristate "Allwinner sunxi SD/MMC Host Controller support"
depends on ARCH_SUNXI
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 3595f83..26ddf76 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_MMC_VUB300) += vub300.o
obj-$(CONFIG_MMC_USHC) += ushc.o
obj-$(CONFIG_MMC_WMT) += wmt-sdmmc.o
obj-$(CONFIG_MMC_MOXART) += moxart-mmc.o
+obj-$(CONFIG_MMC_MESON) += meson-mmc.o
obj-$(CONFIG_MMC_SUNXI) += sunxi-mmc.o
obj-$(CONFIG_MMC_USDHI6ROL0) += usdhi6rol0.o
obj-$(CONFIG_MMC_TOSHIBA_PCI) += toshsd.o
diff --git a/drivers/mmc/host/meson-mmc.c b/drivers/mmc/host/meson-mmc.c
new file mode 100644
index 0000000..6382470
--- /dev/null
+++ b/drivers/mmc/host/meson-mmc.c
@@ -0,0 +1,527 @@
+/*
+ * meson-mmc.c - Meson SDH Controller
+ *
+ * Copyright (C) 2015 Endless Mobile, Inc.
+ * Author: Carlo Caione <carlo@endlessm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/ioport.h>
+#include <linux/platform_device.h>
+
+#include <linux/mmc/host.h>
+#include <linux/mmc/mmc.h>
+#include <linux/mmc/sdio.h>
+#include <linux/mmc/slot-gpio.h>
+
+#include <linux/of_address.h>
+#include <linux/of_gpio.h>
+#include <linux/of_platform.h>
+
+#define SDIO_ARGU (0x00)
+#define SDIO_SEND (0x04)
+#define SDIO_CONF (0x08)
+#define SDIO_IRQS (0x0c)
+#define SDIO_IRQC (0x10)
+#define SDIO_MULT (0x14)
+#define SDIO_ADDR (0x18)
+#define SDIO_EXT (0x1c)
+
+#define REG_IRQS_RESP_CRC7 BIT(5)
+#define REG_IRQS_RD_CRC16 BIT(6)
+#define REG_IRQS_WR_CRC16 BIT(7)
+#define REG_IRQS_CMD_INT BIT(9)
+
+#define REG_IRQC_ARC_CMD_INT BIT(4)
+
+#define REG_CONF_CLK_DIV_M (0x3ff)
+#define REG_CONF_BUS_WIDTH BIT(20)
+
+#define REG_MULT_PORT_SEL_M (0x3)
+#define REG_MULT_RD_INDEX_M (0x0f)
+#define REG_MULT_RD_INDEX_S (12)
+#define REG_MULT_WR_RD_OUT_IND BIT(8)
+
+#define REG_SEND_CMD_COMMAND_M (0xff)
+#define REG_SEND_CMD_RESP_S (8)
+#define REG_SEND_RESP_NO_CRC7 BIT(16)
+#define REG_SEND_RESP_HAVE_DATA BIT(17)
+#define REG_SEND_RESP_CRC7_F_8 BIT(18)
+#define REG_SEND_CHECK_BUSY_D0 BIT(19)
+#define REG_SEND_CMD_SEND_DATA BIT(20)
+#define REG_SEND_REP_PACK_N_M (0xff)
+#define REG_SEND_REP_PACK_N_S (24)
+
+#define REG_EXT_DAT_RW_NUM_M (0x3fff)
+#define REG_EXT_DAT_RW_NUM_S (16)
+
+#define SDIO_BOUNCE_REQ_SIZE (128 * 1024)
+
+struct meson_mmc_host {
+ struct mmc_host *mmc;
+ struct mmc_request *mrq;
+ struct delayed_work timeout_work;
+ struct clk *clk;
+ spinlock_t lock;
+ void __iomem *base;
+ long timeout;
+ int irq;
+ int error;
+ unsigned int port;
+ bool cmd_is_stop;
+ bool dying;
+};
+
+static int meson_mmc_clk_set_rate(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+ struct meson_mmc_host *host = mmc_priv(mmc);
+ unsigned long clk_rate;
+ unsigned int clk_ios = ios->clock;
+ unsigned int clk_div;
+ u32 conf_reg;
+
+ clk_rate = clk_get_rate(host->clk);
+ if (clk_rate < 0) {
+ dev_err(mmc_dev(mmc), "cannot get clock rate\n");
+ return -EINVAL;
+ }
+
+ clk_rate >>= 1;
+ clk_div = clk_rate / clk_ios - !(clk_rate % clk_ios);
+
+ conf_reg = readl(host->base + SDIO_CONF);
+ conf_reg &= ~REG_CONF_CLK_DIV_M;
+ conf_reg |= clk_div;
+ writel(conf_reg, host->base + SDIO_CONF);
+
+ dev_dbg(mmc_dev(mmc), "clk_ios: %d, clk_div: %d, clk_rate: %ld\n",
+ clk_ios, clk_div, clk_rate);
+
+ return 0;
+}
+
+static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+ struct meson_mmc_host *host = mmc_priv(mmc);
+ u32 reg;
+
+ reg = readl(host->base + SDIO_CONF);
+
+ switch (ios->bus_width) {
+ case MMC_BUS_WIDTH_1:
+ reg &= ~REG_CONF_BUS_WIDTH;
+ break;
+ case MMC_BUS_WIDTH_4:
+ reg |= REG_CONF_BUS_WIDTH;
+ break;
+ case MMC_BUS_WIDTH_8:
+ default:
+ dev_err(mmc_dev(mmc), "meson controller doesn't support 8bit data bus\n");
+ host->error = -EINVAL;
+ return;
+ }
+
+ writel(reg, host->base + SDIO_CONF);
+
+ if (ios->clock && ios->power_mode)
+ host->error = meson_mmc_clk_set_rate(mmc, ios);
+}
+
+static void meson_mmc_start_cmd(struct mmc_host *mmc,
+ struct mmc_command *cmd)
+{
+ struct meson_mmc_host *host = mmc_priv(mmc);
+ unsigned int pack_size;
+ u32 irqc, irqs, mult;
+ u32 send = 0;
+ u32 ext = 0;
+
+ switch (mmc_resp_type(cmd)) {
+ case MMC_RSP_R1:
+ case MMC_RSP_R1B:
+ case MMC_RSP_R3:
+ send |= (45 << REG_SEND_CMD_RESP_S);
+ break;
+ case MMC_RSP_R2:
+ send |= (133 << REG_SEND_CMD_RESP_S);
+ send |= REG_SEND_RESP_CRC7_F_8;
+ break;
+ default:
+ break;
+ }
+
+ if (!(cmd->flags & MMC_RSP_CRC))
+ send |= REG_SEND_RESP_NO_CRC7;
+
+ if (cmd->flags & MMC_RSP_BUSY)
+ send |= REG_SEND_CHECK_BUSY_D0;
+
+ if (cmd->data) {
+ send &= ~(REG_SEND_REP_PACK_N_M << REG_SEND_REP_PACK_N_S);
+ send |= ((cmd->data->blocks - 1) << REG_SEND_REP_PACK_N_S);
+
+ ext &= ~(REG_EXT_DAT_RW_NUM_M << REG_EXT_DAT_RW_NUM_S);
+ if (mmc->ios.bus_width)
+ pack_size = cmd->data->blksz * 8 + (16 - 1) * 4;
+ else
+ pack_size = cmd->data->blksz * 8 + (16 - 1);
+ ext |= (pack_size << REG_EXT_DAT_RW_NUM_S);
+
+ if (cmd->data->flags & MMC_DATA_WRITE)
+ send |= REG_SEND_CMD_SEND_DATA;
+ else
+ send |= REG_SEND_RESP_HAVE_DATA;
+ }
+
+ send &= ~REG_SEND_CMD_COMMAND_M;
+ send |= (0x40 | cmd->opcode);
+
+ irqc = readl(host->base + SDIO_IRQC);
+ irqc |= REG_IRQC_ARC_CMD_INT;
+
+ irqs = readl(host->base + SDIO_IRQS);
+ irqs |= REG_IRQS_CMD_INT;
+
+ mult = readl(host->base + SDIO_MULT);
+ mult &= ~REG_MULT_PORT_SEL_M;
+ mult |= host->port;
+ mult |= (1 << 31);
+ writel(mult, host->base + SDIO_MULT);
+ writel(irqs, host->base + SDIO_IRQS);
+ writel(irqc, host->base + SDIO_IRQC);
+
+ writel(cmd->arg, host->base + SDIO_ARGU);
+ writel(ext, host->base + SDIO_EXT);
+ writel(send, host->base + SDIO_SEND);
+}
+
+static int meson_mmc_map_dma(struct meson_mmc_host *host,
+ struct mmc_data *data,
+ unsigned int flags)
+{ u32 dma_len;
+ struct scatterlist *sg = data->sg;
+
+ if (sg->offset & 3 || sg->length & 3) {
+ dev_err(mmc_dev(host->mmc),
+ "unaligned scatterlist: os %x length %d\n",
+ sg->offset, sg->length);
+ return -EINVAL;
+ }
+
+ dma_len = dma_map_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
+ ((data->flags & MMC_DATA_READ) ?
+ DMA_FROM_DEVICE : DMA_TO_DEVICE));
+ if (dma_len == 0) {
+ dev_err(mmc_dev(host->mmc), "dma_map_sg failed\n");
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
+static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
+{
+ struct meson_mmc_host *host = mmc_priv(mmc);
+ struct mmc_command *cmd = mrq->cmd;
+ struct mmc_data *data = mrq->data;
+ unsigned long flags;
+ int ret;
+
+ spin_lock_irqsave(&host->lock, flags);
+
+ if (host->error) {
+ cmd->error = host->error;
+ spin_unlock_irqrestore(&host->lock, flags);
+ mmc_request_done(mmc, mrq);
+ return;
+ }
+
+ if (data) {
+ ret = meson_mmc_map_dma(host, data, data->flags);
+ if (ret < 0) {
+ dev_err(mmc_dev(mmc), "map DMA failed\n");
+ cmd->error = ret;
+ data->error = ret;
+ spin_unlock_irqrestore(&host->lock, flags);
+ mmc_request_done(mmc, mrq);
+ return;
+ }
+ writel(sg_dma_address(data->sg), host->base + SDIO_ADDR);
+ }
+
+ host->mrq = mrq;
+ meson_mmc_start_cmd(mmc, mrq->cmd);
+
+ spin_unlock_irqrestore(&host->lock, flags);
+
+ schedule_delayed_work(&host->timeout_work, host->timeout);
+}
+
+static irqreturn_t meson_mmc_irq(int irq, void *data)
+{
+ struct meson_mmc_host *host = (void *) data;
+ struct mmc_request *mrq = host->mrq;
+ u32 irqs;
+
+ irqs = readl(host->base + SDIO_IRQS);
+ if (mrq && (irqs & REG_IRQS_CMD_INT))
+ return IRQ_WAKE_THREAD;
+
+ return IRQ_HANDLED;
+}
+
+void meson_mmc_read_response(struct meson_mmc_host *host)
+{
+ struct mmc_command *cmd = host->mrq->cmd;
+ u32 mult;
+ int i, resp[4] = { 0 };
+
+ mult = readl(host->base + SDIO_MULT);
+ mult |= REG_MULT_WR_RD_OUT_IND;
+ mult &= ~(REG_MULT_RD_INDEX_M << REG_MULT_RD_INDEX_S);
+ writel(mult, host->base + SDIO_MULT);
+
+ if (cmd->flags & MMC_RSP_136) {
+ for (i = 0; i <= 3; i++)
+ resp[3 - i] = readl(host->base + SDIO_ARGU);
+ cmd->resp[0] = (resp[0] << 8) | ((resp[1] >> 24) & 0xff);
+ cmd->resp[1] = (resp[1] << 8) | ((resp[2] >> 24) & 0xff);
+ cmd->resp[2] = (resp[2] << 8) | ((resp[3] >> 24) & 0xff);
+ cmd->resp[3] = (resp[3] << 8);
+ } else if (cmd->flags & MMC_RSP_PRESENT) {
+ cmd->resp[0] = readl(host->base + SDIO_ARGU);
+ }
+}
+
+static irqreturn_t meson_mmc_irq_thread(int irq, void *irq_data)
+{
+ struct meson_mmc_host *host = (void *) irq_data;
+ struct mmc_data *data;
+ unsigned long flags;
+ struct mmc_request *mrq;
+ u32 irqs, send;
+
+ cancel_delayed_work_sync(&host->timeout_work);
+ spin_lock_irqsave(&host->lock, flags);
+
+ mrq = host->mrq;
+ data = mrq->data;
+
+ if (!mrq) {
+ spin_unlock_irqrestore(&host->lock, flags);
+ return IRQ_HANDLED;
+ }
+
+ if (host->cmd_is_stop)
+ goto out;
+
+ irqs = readl(host->base + SDIO_IRQS);
+ send = readl(host->base + SDIO_SEND);
+
+ mrq->cmd->error = 0;
+
+ if (!data) {
+ if (!((irqs & REG_IRQS_RESP_CRC7) ||
+ (send & REG_SEND_RESP_NO_CRC7)))
+ mrq->cmd->error = -EILSEQ;
+ else
+ meson_mmc_read_response(host);
+ } else {
+ if (!((irqs & REG_IRQS_RD_CRC16) ||
+ (irqs & REG_IRQS_WR_CRC16))) {
+ mrq->cmd->error = -EILSEQ;
+ } else {
+ data->bytes_xfered = data->blksz * data->blocks;
+ dma_unmap_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
+ ((data->flags & MMC_DATA_READ) ?
+ DMA_FROM_DEVICE : DMA_TO_DEVICE));
+ }
+ }
+
+ if (mrq->stop) {
+ host->cmd_is_stop = true;
+ meson_mmc_start_cmd(host->mmc, mrq->stop);
+ spin_unlock_irqrestore(&host->lock, flags);
+ return IRQ_HANDLED;
+ }
+
+out:
+ host->cmd_is_stop = false;
+ host->mrq = NULL;
+ spin_unlock_irqrestore(&host->lock, flags);
+ mmc_request_done(host->mmc, mrq);
+
+ return IRQ_HANDLED;
+}
+
+static void meson_mmc_timeout(struct work_struct *work)
+{
+ struct meson_mmc_host *host = container_of(work,
+ struct meson_mmc_host,
+ timeout_work.work);
+ struct mmc_request *mrq = host->mrq;
+ unsigned long flags;
+ u32 irqc;
+
+ /* Do not run after meson_mmc_remove() */
+ if (host->dying)
+ return;
+
+ spin_lock_irqsave(&host->lock, flags);
+
+ dev_err(mmc_dev(host->mmc), "Timeout on CMD%u\n", mrq->cmd->opcode);
+
+ irqc = readl(host->base + SDIO_IRQC);
+ irqc &= ~REG_IRQC_ARC_CMD_INT;
+ writel(irqc, host->base + SDIO_IRQC);
+
+ mrq->cmd->error = -ETIMEDOUT;
+
+ host->mrq = NULL;
+ spin_unlock_irqrestore(&host->lock, flags);
+
+ mmc_request_done(host->mmc, mrq);
+}
+
+static struct mmc_host_ops meson_mmc_ops = {
+ .request = meson_mmc_request,
+ .set_ios = meson_mmc_set_ios,
+ .get_cd = mmc_gpio_get_cd,
+};
+
+static int meson_mmc_probe(struct platform_device *pdev)
+{
+ struct mmc_host *mmc;
+ struct meson_mmc_host *host;
+ struct pinctrl *pinctrl;
+ struct pinctrl_state *pins;
+ struct resource *res;
+ int ret, irq;
+
+ mmc = mmc_alloc_host(sizeof(struct meson_mmc_host), &pdev->dev);
+ if (!mmc) {
+ dev_err(&pdev->dev, "mmc alloc host failed\n");
+ return -ENOMEM;
+ }
+
+ host = mmc_priv(mmc);
+ host->mmc = mmc;
+ host->timeout = msecs_to_jiffies(2000);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ host->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(host->base)) {
+ ret = PTR_ERR(host->base);
+ goto error_free_host;
+ }
+
+ irq = platform_get_irq(pdev, 0);
+ ret = devm_request_threaded_irq(&pdev->dev, irq, meson_mmc_irq,
+ meson_mmc_irq_thread, 0, "meson_mmc",
+ host);
+ if (ret)
+ goto error_free_host;
+
+ host->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(host->clk)) {
+ ret = PTR_ERR(host->clk);
+ goto error_free_host;
+ }
+
+ /* we do not support scatter lists in hardware */
+ mmc->max_segs = 1;
+ mmc->max_req_size = SDIO_BOUNCE_REQ_SIZE;
+ mmc->max_seg_size = mmc->max_req_size;
+ mmc->max_blk_count = 256;
+ mmc->max_blk_size = mmc->max_req_size / mmc->max_blk_count;
+ mmc->f_min = 300000;
+ mmc->f_max = 50000000;
+ mmc->caps |= MMC_CAP_4_BIT_DATA;
+ mmc->caps |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED;
+ mmc->caps2 |= MMC_CAP2_NO_SDIO;
+ mmc->ocr_avail = MMC_VDD_33_34;
+ mmc->ops = &meson_mmc_ops;
+
+ spin_lock_init(&host->lock);
+
+ INIT_DELAYED_WORK(&host->timeout_work, meson_mmc_timeout);
+
+ pinctrl = devm_pinctrl_get(&pdev->dev);
+ if (IS_ERR(pinctrl)) {
+ ret = PTR_ERR(pinctrl);
+ goto error_free_host;
+ }
+
+ /* We need to know at runtime which port we are using */
+ pins = pinctrl_lookup_state(pinctrl, "sdio_b");
+ if (!IS_ERR(pins))
+ host->port = 1; /* PORT_B */
+ else
+ host->port = 0; /* PORT_A */
+
+ ret = mmc_of_parse(mmc);
+ if (ret)
+ goto error_free_host;
+
+ platform_set_drvdata(pdev, mmc);
+
+ ret = mmc_add_host(mmc);
+ if (ret)
+ goto error_free_host;
+
+ dev_info(&pdev->dev, "base:0x%p irq:%u port:%u\n",
+ host->base, irq, host->port);
+
+ return 0;
+
+error_free_host:
+ mmc_free_host(mmc);
+
+ return ret;
+}
+
+static int meson_mmc_remove(struct platform_device *pdev)
+{
+ struct mmc_host *mmc = platform_get_drvdata(pdev);
+ struct meson_mmc_host *host = mmc_priv(mmc);
+
+ host->dying = true;
+
+ mmc_remove_host(mmc);
+ cancel_delayed_work_sync(&host->timeout_work);
+
+ mmc_free_host(mmc);
+
+ return 0;
+}
+
+static const struct of_device_id meson_mmc_of_match[] = {
+ { .compatible = "amlogic,meson-mmc", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, meson_mmc_of_match);
+
+static struct platform_driver meson_mmc_driver = {
+ .probe = meson_mmc_probe,
+ .remove = meson_mmc_remove,
+ .driver = {
+ .name = "meson-mmc",
+ .of_match_table = of_match_ptr(meson_mmc_of_match),
+ },
+};
+
+module_platform_driver(meson_mmc_driver);
+
+MODULE_DESCRIPTION("Meson Secure Digital Host Driver");
+MODULE_AUTHOR("Carlo Caione <carlo@endlessm.com>");
+MODULE_LICENSE("GPL");
--
2.5.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
@ 2015-12-01 16:41 ` Carlo Caione
0 siblings, 0 replies; 22+ messages in thread
From: Carlo Caione @ 2015-12-01 16:41 UTC (permalink / raw)
To: linux-arm-kernel
From: Carlo Caione <carlo@endlessm.com>
Add a driver for the SD/MMC host found on the Amlogic MesonX SoCs. This
is an MMC controller which provides an interface between the application
processor and various memory cards. It supports the SD specification
v2.0 and the eMMC specification v4.41.
This patch adds also the binding documentation.
Signed-off-by: Carlo Caione <carlo@endlessm.com>
---
Changelog:
v2:
* fix typo in commit message
* avoid the bounce buffer (max_segs == 1)
v3:
* simplify meson_mmc_map_dma() and fix memory leak
v4:
* code and documentation cleanup
* removed redundant variables
* removed useless printing
* introduced MMC_CAP2_NO_SDIO
* removed state machine
* better spinlocks management and positioning
---
.../devicetree/bindings/mmc/meson-mmc.txt | 29 ++
drivers/mmc/host/Kconfig | 7 +
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/meson-mmc.c | 527 +++++++++++++++++++++
4 files changed, 564 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mmc/meson-mmc.txt
create mode 100644 drivers/mmc/host/meson-mmc.c
diff --git a/Documentation/devicetree/bindings/mmc/meson-mmc.txt b/Documentation/devicetree/bindings/mmc/meson-mmc.txt
new file mode 100644
index 0000000..7ff9e9e
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/meson-mmc.txt
@@ -0,0 +1,29 @@
+* Amlogic MesonX MMC controller
+
+The highspeed MMC host controller on Amlogic SoCs provides an interface
+for MMC, SD, SDIO and SDHC types of memory cards.
+
+Supported maximum speeds are the ones of the eMMC standard 4.41 as well
+as the speed of SD standard 2.0.
+
+Required properties:
+ - compatible : "amlogic,meson-mmc"
+ - reg : mmc controller base registers
+ - interrupts : mmc controller interrupt
+ - clocks : phandle to clock provider
+ - pinctrl-names : should contain "sdio_a" or "sdio_b"
+ - pinctrl-0: Should specify pin control groups used for this controller
+
+Optional properties:
+ - for cd, bus-width and additional generic mmc parameters
+ please refer to mmc.txt within this directory
+
+Examples:
+ mmc0: mmc at c1108c20 {
+ pinctrl-names = "sdio_b";
+ pinctrl-0 = <&mmc0_sd_b_pins>;
+ compatible = "amlogic,meson-mmc";
+ reg = <0xc1108c20 0x20>;
+ interrupts = <0 28 1>;
+ clocks = <&clkc CLKID_CLK81>;
+ };
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 1dee533..7f2c5ae 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -765,6 +765,13 @@ config MMC_REALTEK_USB
Say Y here to include driver code to support SD/MMC card interface
of Realtek RTS5129/39 series card reader
+config MMC_MESON
+ tristate "Amlogic MesonX SD/MMC Host Controller support"
+ depends on ARCH_MESON
+ help
+ This selects support for the SD/MMC Host Controller on
+ Amlogic MesonX SoCs.
+
config MMC_SUNXI
tristate "Allwinner sunxi SD/MMC Host Controller support"
depends on ARCH_SUNXI
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 3595f83..26ddf76 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_MMC_VUB300) += vub300.o
obj-$(CONFIG_MMC_USHC) += ushc.o
obj-$(CONFIG_MMC_WMT) += wmt-sdmmc.o
obj-$(CONFIG_MMC_MOXART) += moxart-mmc.o
+obj-$(CONFIG_MMC_MESON) += meson-mmc.o
obj-$(CONFIG_MMC_SUNXI) += sunxi-mmc.o
obj-$(CONFIG_MMC_USDHI6ROL0) += usdhi6rol0.o
obj-$(CONFIG_MMC_TOSHIBA_PCI) += toshsd.o
diff --git a/drivers/mmc/host/meson-mmc.c b/drivers/mmc/host/meson-mmc.c
new file mode 100644
index 0000000..6382470
--- /dev/null
+++ b/drivers/mmc/host/meson-mmc.c
@@ -0,0 +1,527 @@
+/*
+ * meson-mmc.c - Meson SDH Controller
+ *
+ * Copyright (C) 2015 Endless Mobile, Inc.
+ * Author: Carlo Caione <carlo@endlessm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/ioport.h>
+#include <linux/platform_device.h>
+
+#include <linux/mmc/host.h>
+#include <linux/mmc/mmc.h>
+#include <linux/mmc/sdio.h>
+#include <linux/mmc/slot-gpio.h>
+
+#include <linux/of_address.h>
+#include <linux/of_gpio.h>
+#include <linux/of_platform.h>
+
+#define SDIO_ARGU (0x00)
+#define SDIO_SEND (0x04)
+#define SDIO_CONF (0x08)
+#define SDIO_IRQS (0x0c)
+#define SDIO_IRQC (0x10)
+#define SDIO_MULT (0x14)
+#define SDIO_ADDR (0x18)
+#define SDIO_EXT (0x1c)
+
+#define REG_IRQS_RESP_CRC7 BIT(5)
+#define REG_IRQS_RD_CRC16 BIT(6)
+#define REG_IRQS_WR_CRC16 BIT(7)
+#define REG_IRQS_CMD_INT BIT(9)
+
+#define REG_IRQC_ARC_CMD_INT BIT(4)
+
+#define REG_CONF_CLK_DIV_M (0x3ff)
+#define REG_CONF_BUS_WIDTH BIT(20)
+
+#define REG_MULT_PORT_SEL_M (0x3)
+#define REG_MULT_RD_INDEX_M (0x0f)
+#define REG_MULT_RD_INDEX_S (12)
+#define REG_MULT_WR_RD_OUT_IND BIT(8)
+
+#define REG_SEND_CMD_COMMAND_M (0xff)
+#define REG_SEND_CMD_RESP_S (8)
+#define REG_SEND_RESP_NO_CRC7 BIT(16)
+#define REG_SEND_RESP_HAVE_DATA BIT(17)
+#define REG_SEND_RESP_CRC7_F_8 BIT(18)
+#define REG_SEND_CHECK_BUSY_D0 BIT(19)
+#define REG_SEND_CMD_SEND_DATA BIT(20)
+#define REG_SEND_REP_PACK_N_M (0xff)
+#define REG_SEND_REP_PACK_N_S (24)
+
+#define REG_EXT_DAT_RW_NUM_M (0x3fff)
+#define REG_EXT_DAT_RW_NUM_S (16)
+
+#define SDIO_BOUNCE_REQ_SIZE (128 * 1024)
+
+struct meson_mmc_host {
+ struct mmc_host *mmc;
+ struct mmc_request *mrq;
+ struct delayed_work timeout_work;
+ struct clk *clk;
+ spinlock_t lock;
+ void __iomem *base;
+ long timeout;
+ int irq;
+ int error;
+ unsigned int port;
+ bool cmd_is_stop;
+ bool dying;
+};
+
+static int meson_mmc_clk_set_rate(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+ struct meson_mmc_host *host = mmc_priv(mmc);
+ unsigned long clk_rate;
+ unsigned int clk_ios = ios->clock;
+ unsigned int clk_div;
+ u32 conf_reg;
+
+ clk_rate = clk_get_rate(host->clk);
+ if (clk_rate < 0) {
+ dev_err(mmc_dev(mmc), "cannot get clock rate\n");
+ return -EINVAL;
+ }
+
+ clk_rate >>= 1;
+ clk_div = clk_rate / clk_ios - !(clk_rate % clk_ios);
+
+ conf_reg = readl(host->base + SDIO_CONF);
+ conf_reg &= ~REG_CONF_CLK_DIV_M;
+ conf_reg |= clk_div;
+ writel(conf_reg, host->base + SDIO_CONF);
+
+ dev_dbg(mmc_dev(mmc), "clk_ios: %d, clk_div: %d, clk_rate: %ld\n",
+ clk_ios, clk_div, clk_rate);
+
+ return 0;
+}
+
+static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+ struct meson_mmc_host *host = mmc_priv(mmc);
+ u32 reg;
+
+ reg = readl(host->base + SDIO_CONF);
+
+ switch (ios->bus_width) {
+ case MMC_BUS_WIDTH_1:
+ reg &= ~REG_CONF_BUS_WIDTH;
+ break;
+ case MMC_BUS_WIDTH_4:
+ reg |= REG_CONF_BUS_WIDTH;
+ break;
+ case MMC_BUS_WIDTH_8:
+ default:
+ dev_err(mmc_dev(mmc), "meson controller doesn't support 8bit data bus\n");
+ host->error = -EINVAL;
+ return;
+ }
+
+ writel(reg, host->base + SDIO_CONF);
+
+ if (ios->clock && ios->power_mode)
+ host->error = meson_mmc_clk_set_rate(mmc, ios);
+}
+
+static void meson_mmc_start_cmd(struct mmc_host *mmc,
+ struct mmc_command *cmd)
+{
+ struct meson_mmc_host *host = mmc_priv(mmc);
+ unsigned int pack_size;
+ u32 irqc, irqs, mult;
+ u32 send = 0;
+ u32 ext = 0;
+
+ switch (mmc_resp_type(cmd)) {
+ case MMC_RSP_R1:
+ case MMC_RSP_R1B:
+ case MMC_RSP_R3:
+ send |= (45 << REG_SEND_CMD_RESP_S);
+ break;
+ case MMC_RSP_R2:
+ send |= (133 << REG_SEND_CMD_RESP_S);
+ send |= REG_SEND_RESP_CRC7_F_8;
+ break;
+ default:
+ break;
+ }
+
+ if (!(cmd->flags & MMC_RSP_CRC))
+ send |= REG_SEND_RESP_NO_CRC7;
+
+ if (cmd->flags & MMC_RSP_BUSY)
+ send |= REG_SEND_CHECK_BUSY_D0;
+
+ if (cmd->data) {
+ send &= ~(REG_SEND_REP_PACK_N_M << REG_SEND_REP_PACK_N_S);
+ send |= ((cmd->data->blocks - 1) << REG_SEND_REP_PACK_N_S);
+
+ ext &= ~(REG_EXT_DAT_RW_NUM_M << REG_EXT_DAT_RW_NUM_S);
+ if (mmc->ios.bus_width)
+ pack_size = cmd->data->blksz * 8 + (16 - 1) * 4;
+ else
+ pack_size = cmd->data->blksz * 8 + (16 - 1);
+ ext |= (pack_size << REG_EXT_DAT_RW_NUM_S);
+
+ if (cmd->data->flags & MMC_DATA_WRITE)
+ send |= REG_SEND_CMD_SEND_DATA;
+ else
+ send |= REG_SEND_RESP_HAVE_DATA;
+ }
+
+ send &= ~REG_SEND_CMD_COMMAND_M;
+ send |= (0x40 | cmd->opcode);
+
+ irqc = readl(host->base + SDIO_IRQC);
+ irqc |= REG_IRQC_ARC_CMD_INT;
+
+ irqs = readl(host->base + SDIO_IRQS);
+ irqs |= REG_IRQS_CMD_INT;
+
+ mult = readl(host->base + SDIO_MULT);
+ mult &= ~REG_MULT_PORT_SEL_M;
+ mult |= host->port;
+ mult |= (1 << 31);
+ writel(mult, host->base + SDIO_MULT);
+ writel(irqs, host->base + SDIO_IRQS);
+ writel(irqc, host->base + SDIO_IRQC);
+
+ writel(cmd->arg, host->base + SDIO_ARGU);
+ writel(ext, host->base + SDIO_EXT);
+ writel(send, host->base + SDIO_SEND);
+}
+
+static int meson_mmc_map_dma(struct meson_mmc_host *host,
+ struct mmc_data *data,
+ unsigned int flags)
+{ u32 dma_len;
+ struct scatterlist *sg = data->sg;
+
+ if (sg->offset & 3 || sg->length & 3) {
+ dev_err(mmc_dev(host->mmc),
+ "unaligned scatterlist: os %x length %d\n",
+ sg->offset, sg->length);
+ return -EINVAL;
+ }
+
+ dma_len = dma_map_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
+ ((data->flags & MMC_DATA_READ) ?
+ DMA_FROM_DEVICE : DMA_TO_DEVICE));
+ if (dma_len == 0) {
+ dev_err(mmc_dev(host->mmc), "dma_map_sg failed\n");
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
+static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
+{
+ struct meson_mmc_host *host = mmc_priv(mmc);
+ struct mmc_command *cmd = mrq->cmd;
+ struct mmc_data *data = mrq->data;
+ unsigned long flags;
+ int ret;
+
+ spin_lock_irqsave(&host->lock, flags);
+
+ if (host->error) {
+ cmd->error = host->error;
+ spin_unlock_irqrestore(&host->lock, flags);
+ mmc_request_done(mmc, mrq);
+ return;
+ }
+
+ if (data) {
+ ret = meson_mmc_map_dma(host, data, data->flags);
+ if (ret < 0) {
+ dev_err(mmc_dev(mmc), "map DMA failed\n");
+ cmd->error = ret;
+ data->error = ret;
+ spin_unlock_irqrestore(&host->lock, flags);
+ mmc_request_done(mmc, mrq);
+ return;
+ }
+ writel(sg_dma_address(data->sg), host->base + SDIO_ADDR);
+ }
+
+ host->mrq = mrq;
+ meson_mmc_start_cmd(mmc, mrq->cmd);
+
+ spin_unlock_irqrestore(&host->lock, flags);
+
+ schedule_delayed_work(&host->timeout_work, host->timeout);
+}
+
+static irqreturn_t meson_mmc_irq(int irq, void *data)
+{
+ struct meson_mmc_host *host = (void *) data;
+ struct mmc_request *mrq = host->mrq;
+ u32 irqs;
+
+ irqs = readl(host->base + SDIO_IRQS);
+ if (mrq && (irqs & REG_IRQS_CMD_INT))
+ return IRQ_WAKE_THREAD;
+
+ return IRQ_HANDLED;
+}
+
+void meson_mmc_read_response(struct meson_mmc_host *host)
+{
+ struct mmc_command *cmd = host->mrq->cmd;
+ u32 mult;
+ int i, resp[4] = { 0 };
+
+ mult = readl(host->base + SDIO_MULT);
+ mult |= REG_MULT_WR_RD_OUT_IND;
+ mult &= ~(REG_MULT_RD_INDEX_M << REG_MULT_RD_INDEX_S);
+ writel(mult, host->base + SDIO_MULT);
+
+ if (cmd->flags & MMC_RSP_136) {
+ for (i = 0; i <= 3; i++)
+ resp[3 - i] = readl(host->base + SDIO_ARGU);
+ cmd->resp[0] = (resp[0] << 8) | ((resp[1] >> 24) & 0xff);
+ cmd->resp[1] = (resp[1] << 8) | ((resp[2] >> 24) & 0xff);
+ cmd->resp[2] = (resp[2] << 8) | ((resp[3] >> 24) & 0xff);
+ cmd->resp[3] = (resp[3] << 8);
+ } else if (cmd->flags & MMC_RSP_PRESENT) {
+ cmd->resp[0] = readl(host->base + SDIO_ARGU);
+ }
+}
+
+static irqreturn_t meson_mmc_irq_thread(int irq, void *irq_data)
+{
+ struct meson_mmc_host *host = (void *) irq_data;
+ struct mmc_data *data;
+ unsigned long flags;
+ struct mmc_request *mrq;
+ u32 irqs, send;
+
+ cancel_delayed_work_sync(&host->timeout_work);
+ spin_lock_irqsave(&host->lock, flags);
+
+ mrq = host->mrq;
+ data = mrq->data;
+
+ if (!mrq) {
+ spin_unlock_irqrestore(&host->lock, flags);
+ return IRQ_HANDLED;
+ }
+
+ if (host->cmd_is_stop)
+ goto out;
+
+ irqs = readl(host->base + SDIO_IRQS);
+ send = readl(host->base + SDIO_SEND);
+
+ mrq->cmd->error = 0;
+
+ if (!data) {
+ if (!((irqs & REG_IRQS_RESP_CRC7) ||
+ (send & REG_SEND_RESP_NO_CRC7)))
+ mrq->cmd->error = -EILSEQ;
+ else
+ meson_mmc_read_response(host);
+ } else {
+ if (!((irqs & REG_IRQS_RD_CRC16) ||
+ (irqs & REG_IRQS_WR_CRC16))) {
+ mrq->cmd->error = -EILSEQ;
+ } else {
+ data->bytes_xfered = data->blksz * data->blocks;
+ dma_unmap_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
+ ((data->flags & MMC_DATA_READ) ?
+ DMA_FROM_DEVICE : DMA_TO_DEVICE));
+ }
+ }
+
+ if (mrq->stop) {
+ host->cmd_is_stop = true;
+ meson_mmc_start_cmd(host->mmc, mrq->stop);
+ spin_unlock_irqrestore(&host->lock, flags);
+ return IRQ_HANDLED;
+ }
+
+out:
+ host->cmd_is_stop = false;
+ host->mrq = NULL;
+ spin_unlock_irqrestore(&host->lock, flags);
+ mmc_request_done(host->mmc, mrq);
+
+ return IRQ_HANDLED;
+}
+
+static void meson_mmc_timeout(struct work_struct *work)
+{
+ struct meson_mmc_host *host = container_of(work,
+ struct meson_mmc_host,
+ timeout_work.work);
+ struct mmc_request *mrq = host->mrq;
+ unsigned long flags;
+ u32 irqc;
+
+ /* Do not run after meson_mmc_remove() */
+ if (host->dying)
+ return;
+
+ spin_lock_irqsave(&host->lock, flags);
+
+ dev_err(mmc_dev(host->mmc), "Timeout on CMD%u\n", mrq->cmd->opcode);
+
+ irqc = readl(host->base + SDIO_IRQC);
+ irqc &= ~REG_IRQC_ARC_CMD_INT;
+ writel(irqc, host->base + SDIO_IRQC);
+
+ mrq->cmd->error = -ETIMEDOUT;
+
+ host->mrq = NULL;
+ spin_unlock_irqrestore(&host->lock, flags);
+
+ mmc_request_done(host->mmc, mrq);
+}
+
+static struct mmc_host_ops meson_mmc_ops = {
+ .request = meson_mmc_request,
+ .set_ios = meson_mmc_set_ios,
+ .get_cd = mmc_gpio_get_cd,
+};
+
+static int meson_mmc_probe(struct platform_device *pdev)
+{
+ struct mmc_host *mmc;
+ struct meson_mmc_host *host;
+ struct pinctrl *pinctrl;
+ struct pinctrl_state *pins;
+ struct resource *res;
+ int ret, irq;
+
+ mmc = mmc_alloc_host(sizeof(struct meson_mmc_host), &pdev->dev);
+ if (!mmc) {
+ dev_err(&pdev->dev, "mmc alloc host failed\n");
+ return -ENOMEM;
+ }
+
+ host = mmc_priv(mmc);
+ host->mmc = mmc;
+ host->timeout = msecs_to_jiffies(2000);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ host->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(host->base)) {
+ ret = PTR_ERR(host->base);
+ goto error_free_host;
+ }
+
+ irq = platform_get_irq(pdev, 0);
+ ret = devm_request_threaded_irq(&pdev->dev, irq, meson_mmc_irq,
+ meson_mmc_irq_thread, 0, "meson_mmc",
+ host);
+ if (ret)
+ goto error_free_host;
+
+ host->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(host->clk)) {
+ ret = PTR_ERR(host->clk);
+ goto error_free_host;
+ }
+
+ /* we do not support scatter lists in hardware */
+ mmc->max_segs = 1;
+ mmc->max_req_size = SDIO_BOUNCE_REQ_SIZE;
+ mmc->max_seg_size = mmc->max_req_size;
+ mmc->max_blk_count = 256;
+ mmc->max_blk_size = mmc->max_req_size / mmc->max_blk_count;
+ mmc->f_min = 300000;
+ mmc->f_max = 50000000;
+ mmc->caps |= MMC_CAP_4_BIT_DATA;
+ mmc->caps |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED;
+ mmc->caps2 |= MMC_CAP2_NO_SDIO;
+ mmc->ocr_avail = MMC_VDD_33_34;
+ mmc->ops = &meson_mmc_ops;
+
+ spin_lock_init(&host->lock);
+
+ INIT_DELAYED_WORK(&host->timeout_work, meson_mmc_timeout);
+
+ pinctrl = devm_pinctrl_get(&pdev->dev);
+ if (IS_ERR(pinctrl)) {
+ ret = PTR_ERR(pinctrl);
+ goto error_free_host;
+ }
+
+ /* We need to know at runtime which port we are using */
+ pins = pinctrl_lookup_state(pinctrl, "sdio_b");
+ if (!IS_ERR(pins))
+ host->port = 1; /* PORT_B */
+ else
+ host->port = 0; /* PORT_A */
+
+ ret = mmc_of_parse(mmc);
+ if (ret)
+ goto error_free_host;
+
+ platform_set_drvdata(pdev, mmc);
+
+ ret = mmc_add_host(mmc);
+ if (ret)
+ goto error_free_host;
+
+ dev_info(&pdev->dev, "base:0x%p irq:%u port:%u\n",
+ host->base, irq, host->port);
+
+ return 0;
+
+error_free_host:
+ mmc_free_host(mmc);
+
+ return ret;
+}
+
+static int meson_mmc_remove(struct platform_device *pdev)
+{
+ struct mmc_host *mmc = platform_get_drvdata(pdev);
+ struct meson_mmc_host *host = mmc_priv(mmc);
+
+ host->dying = true;
+
+ mmc_remove_host(mmc);
+ cancel_delayed_work_sync(&host->timeout_work);
+
+ mmc_free_host(mmc);
+
+ return 0;
+}
+
+static const struct of_device_id meson_mmc_of_match[] = {
+ { .compatible = "amlogic,meson-mmc", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, meson_mmc_of_match);
+
+static struct platform_driver meson_mmc_driver = {
+ .probe = meson_mmc_probe,
+ .remove = meson_mmc_remove,
+ .driver = {
+ .name = "meson-mmc",
+ .of_match_table = of_match_ptr(meson_mmc_of_match),
+ },
+};
+
+module_platform_driver(meson_mmc_driver);
+
+MODULE_DESCRIPTION("Meson Secure Digital Host Driver");
+MODULE_AUTHOR("Carlo Caione <carlo@endlessm.com>");
+MODULE_LICENSE("GPL");
--
2.5.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
2015-12-01 16:41 ` Carlo Caione
@ 2015-12-08 15:06 ` Ulf Hansson
-1 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2015-12-08 15:06 UTC (permalink / raw)
To: Carlo Caione
Cc: linux-arm-kernel, linux-mmc, Daniel Drake, jerry.cao, victor.wan,
linux-meson, Carlo Caione
On 1 December 2015 at 17:41, Carlo Caione <carlo@caione.org> wrote:
> From: Carlo Caione <carlo@endlessm.com>
>
> Add a driver for the SD/MMC host found on the Amlogic MesonX SoCs. This
> is an MMC controller which provides an interface between the application
> processor and various memory cards. It supports the SD specification
> v2.0 and the eMMC specification v4.41.
>
> This patch adds also the binding documentation.
>
> Signed-off-by: Carlo Caione <carlo@endlessm.com>
> ---
> Changelog:
>
> v2:
> * fix typo in commit message
> * avoid the bounce buffer (max_segs == 1)
>
> v3:
> * simplify meson_mmc_map_dma() and fix memory leak
>
> v4:
> * code and documentation cleanup
> * removed redundant variables
> * removed useless printing
> * introduced MMC_CAP2_NO_SDIO
> * removed state machine
> * better spinlocks management and positioning
> ---
> .../devicetree/bindings/mmc/meson-mmc.txt | 29 ++
> drivers/mmc/host/Kconfig | 7 +
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/meson-mmc.c | 527 +++++++++++++++++++++
> 4 files changed, 564 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/meson-mmc.txt
> create mode 100644 drivers/mmc/host/meson-mmc.c
>
> diff --git a/Documentation/devicetree/bindings/mmc/meson-mmc.txt b/Documentation/devicetree/bindings/mmc/meson-mmc.txt
> new file mode 100644
> index 0000000..7ff9e9e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/meson-mmc.txt
> @@ -0,0 +1,29 @@
> +* Amlogic MesonX MMC controller
> +
> +The highspeed MMC host controller on Amlogic SoCs provides an interface
> +for MMC, SD, SDIO and SDHC types of memory cards.
> +
> +Supported maximum speeds are the ones of the eMMC standard 4.41 as well
> +as the speed of SD standard 2.0.
> +
> +Required properties:
> + - compatible : "amlogic,meson-mmc"
> + - reg : mmc controller base registers
> + - interrupts : mmc controller interrupt
> + - clocks : phandle to clock provider
> + - pinctrl-names : should contain "sdio_a" or "sdio_b"
This is weird. I don't think you need this specific states.
You should be able to cope with common used "default" state.
Later in the driver code I realize that you have some register bits
that is related to either using port a or port b. But I don't think
that is the same thing as having different pinctrls states.
If you can't detect whether port a or b is used in runtime by the
driver, I suggest you add a separate meson mmc DT binding telling what
port is being used.
> + - pinctrl-0: Should specify pin control groups used for this controller
> +
> +Optional properties:
> + - for cd, bus-width and additional generic mmc parameters
> + please refer to mmc.txt within this directory
> +
> +Examples:
> + mmc0: mmc@c1108c20 {
> + pinctrl-names = "sdio_b";
> + pinctrl-0 = <&mmc0_sd_b_pins>;
> + compatible = "amlogic,meson-mmc";
> + reg = <0xc1108c20 0x20>;
> + interrupts = <0 28 1>;
> + clocks = <&clkc CLKID_CLK81>;
> + };
Overall the DT documentation looks okay (beside the comments above).
Although, please separate it to become a patch 1/2. The rest below
shoud go into a patch 2/2.
For the DT patch please include the DT maintainers when posting the new version.
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 1dee533..7f2c5ae 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -765,6 +765,13 @@ config MMC_REALTEK_USB
> Say Y here to include driver code to support SD/MMC card interface
> of Realtek RTS5129/39 series card reader
>
> +config MMC_MESON
> + tristate "Amlogic MesonX SD/MMC Host Controller support"
> + depends on ARCH_MESON
> + help
> + This selects support for the SD/MMC Host Controller on
> + Amlogic MesonX SoCs.
> +
> config MMC_SUNXI
> tristate "Allwinner sunxi SD/MMC Host Controller support"
> depends on ARCH_SUNXI
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index 3595f83..26ddf76 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -54,6 +54,7 @@ obj-$(CONFIG_MMC_VUB300) += vub300.o
> obj-$(CONFIG_MMC_USHC) += ushc.o
> obj-$(CONFIG_MMC_WMT) += wmt-sdmmc.o
> obj-$(CONFIG_MMC_MOXART) += moxart-mmc.o
> +obj-$(CONFIG_MMC_MESON) += meson-mmc.o
> obj-$(CONFIG_MMC_SUNXI) += sunxi-mmc.o
> obj-$(CONFIG_MMC_USDHI6ROL0) += usdhi6rol0.o
> obj-$(CONFIG_MMC_TOSHIBA_PCI) += toshsd.o
> diff --git a/drivers/mmc/host/meson-mmc.c b/drivers/mmc/host/meson-mmc.c
> new file mode 100644
> index 0000000..6382470
> --- /dev/null
> +++ b/drivers/mmc/host/meson-mmc.c
> @@ -0,0 +1,527 @@
> +/*
> + * meson-mmc.c - Meson SDH Controller
> + *
> + * Copyright (C) 2015 Endless Mobile, Inc.
> + * Author: Carlo Caione <carlo@endlessm.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or (at
> + * your option) any later version.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/ioport.h>
> +#include <linux/platform_device.h>
> +
> +#include <linux/mmc/host.h>
> +#include <linux/mmc/mmc.h>
> +#include <linux/mmc/sdio.h>
> +#include <linux/mmc/slot-gpio.h>
> +
> +#include <linux/of_address.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_platform.h>
> +
> +#define SDIO_ARGU (0x00)
> +#define SDIO_SEND (0x04)
> +#define SDIO_CONF (0x08)
> +#define SDIO_IRQS (0x0c)
> +#define SDIO_IRQC (0x10)
> +#define SDIO_MULT (0x14)
> +#define SDIO_ADDR (0x18)
> +#define SDIO_EXT (0x1c)
> +
> +#define REG_IRQS_RESP_CRC7 BIT(5)
> +#define REG_IRQS_RD_CRC16 BIT(6)
> +#define REG_IRQS_WR_CRC16 BIT(7)
> +#define REG_IRQS_CMD_INT BIT(9)
> +
> +#define REG_IRQC_ARC_CMD_INT BIT(4)
> +
> +#define REG_CONF_CLK_DIV_M (0x3ff)
> +#define REG_CONF_BUS_WIDTH BIT(20)
> +
> +#define REG_MULT_PORT_SEL_M (0x3)
> +#define REG_MULT_RD_INDEX_M (0x0f)
> +#define REG_MULT_RD_INDEX_S (12)
> +#define REG_MULT_WR_RD_OUT_IND BIT(8)
> +
> +#define REG_SEND_CMD_COMMAND_M (0xff)
> +#define REG_SEND_CMD_RESP_S (8)
> +#define REG_SEND_RESP_NO_CRC7 BIT(16)
> +#define REG_SEND_RESP_HAVE_DATA BIT(17)
> +#define REG_SEND_RESP_CRC7_F_8 BIT(18)
> +#define REG_SEND_CHECK_BUSY_D0 BIT(19)
> +#define REG_SEND_CMD_SEND_DATA BIT(20)
> +#define REG_SEND_REP_PACK_N_M (0xff)
> +#define REG_SEND_REP_PACK_N_S (24)
> +
> +#define REG_EXT_DAT_RW_NUM_M (0x3fff)
> +#define REG_EXT_DAT_RW_NUM_S (16)
> +
> +#define SDIO_BOUNCE_REQ_SIZE (128 * 1024)
> +
> +struct meson_mmc_host {
> + struct mmc_host *mmc;
> + struct mmc_request *mrq;
> + struct delayed_work timeout_work;
> + struct clk *clk;
> + spinlock_t lock;
> + void __iomem *base;
> + long timeout;
> + int irq;
> + int error;
> + unsigned int port;
> + bool cmd_is_stop;
> + bool dying;
> +};
> +
> +static int meson_mmc_clk_set_rate(struct mmc_host *mmc, struct mmc_ios *ios)
> +{
> + struct meson_mmc_host *host = mmc_priv(mmc);
> + unsigned long clk_rate;
> + unsigned int clk_ios = ios->clock;
> + unsigned int clk_div;
> + u32 conf_reg;
> +
> + clk_rate = clk_get_rate(host->clk);
> + if (clk_rate < 0) {
> + dev_err(mmc_dev(mmc), "cannot get clock rate\n");
> + return -EINVAL;
> + }
> +
> + clk_rate >>= 1;
> + clk_div = clk_rate / clk_ios - !(clk_rate % clk_ios);
> +
> + conf_reg = readl(host->base + SDIO_CONF);
> + conf_reg &= ~REG_CONF_CLK_DIV_M;
> + conf_reg |= clk_div;
> + writel(conf_reg, host->base + SDIO_CONF);
> +
> + dev_dbg(mmc_dev(mmc), "clk_ios: %d, clk_div: %d, clk_rate: %ld\n",
> + clk_ios, clk_div, clk_rate);
It's good practice to update mmc->actual_clock, as it's being used
from mmc core's debugfs support.
> +
> + return 0;
> +}
> +
> +static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> +{
> + struct meson_mmc_host *host = mmc_priv(mmc);
> + u32 reg;
> +
> + reg = readl(host->base + SDIO_CONF);
> +
> + switch (ios->bus_width) {
> + case MMC_BUS_WIDTH_1:
> + reg &= ~REG_CONF_BUS_WIDTH;
> + break;
> + case MMC_BUS_WIDTH_4:
> + reg |= REG_CONF_BUS_WIDTH;
> + break;
> + case MMC_BUS_WIDTH_8:
> + default:
> + dev_err(mmc_dev(mmc), "meson controller doesn't support 8bit data bus\n");
> + host->error = -EINVAL;
> + return;
> + }
> +
> + writel(reg, host->base + SDIO_CONF);
> +
> + if (ios->clock && ios->power_mode)
> + host->error = meson_mmc_clk_set_rate(mmc, ios);
> +}
> +
> +static void meson_mmc_start_cmd(struct mmc_host *mmc,
> + struct mmc_command *cmd)
> +{
> + struct meson_mmc_host *host = mmc_priv(mmc);
> + unsigned int pack_size;
> + u32 irqc, irqs, mult;
> + u32 send = 0;
> + u32 ext = 0;
> +
> + switch (mmc_resp_type(cmd)) {
> + case MMC_RSP_R1:
> + case MMC_RSP_R1B:
> + case MMC_RSP_R3:
> + send |= (45 << REG_SEND_CMD_RESP_S);
> + break;
> + case MMC_RSP_R2:
> + send |= (133 << REG_SEND_CMD_RESP_S);
> + send |= REG_SEND_RESP_CRC7_F_8;
> + break;
> + default:
> + break;
> + }
> +
> + if (!(cmd->flags & MMC_RSP_CRC))
> + send |= REG_SEND_RESP_NO_CRC7;
> +
> + if (cmd->flags & MMC_RSP_BUSY)
> + send |= REG_SEND_CHECK_BUSY_D0;
> +
> + if (cmd->data) {
> + send &= ~(REG_SEND_REP_PACK_N_M << REG_SEND_REP_PACK_N_S);
> + send |= ((cmd->data->blocks - 1) << REG_SEND_REP_PACK_N_S);
> +
> + ext &= ~(REG_EXT_DAT_RW_NUM_M << REG_EXT_DAT_RW_NUM_S);
> + if (mmc->ios.bus_width)
> + pack_size = cmd->data->blksz * 8 + (16 - 1) * 4;
> + else
> + pack_size = cmd->data->blksz * 8 + (16 - 1);
> + ext |= (pack_size << REG_EXT_DAT_RW_NUM_S);
> +
> + if (cmd->data->flags & MMC_DATA_WRITE)
> + send |= REG_SEND_CMD_SEND_DATA;
> + else
> + send |= REG_SEND_RESP_HAVE_DATA;
> + }
> +
> + send &= ~REG_SEND_CMD_COMMAND_M;
> + send |= (0x40 | cmd->opcode);
> +
> + irqc = readl(host->base + SDIO_IRQC);
> + irqc |= REG_IRQC_ARC_CMD_INT;
> +
> + irqs = readl(host->base + SDIO_IRQS);
> + irqs |= REG_IRQS_CMD_INT;
> +
> + mult = readl(host->base + SDIO_MULT);
> + mult &= ~REG_MULT_PORT_SEL_M;
> + mult |= host->port;
> + mult |= (1 << 31);
> + writel(mult, host->base + SDIO_MULT);
> + writel(irqs, host->base + SDIO_IRQS);
> + writel(irqc, host->base + SDIO_IRQC);
> +
> + writel(cmd->arg, host->base + SDIO_ARGU);
> + writel(ext, host->base + SDIO_EXT);
> + writel(send, host->base + SDIO_SEND);
> +}
> +
> +static int meson_mmc_map_dma(struct meson_mmc_host *host,
> + struct mmc_data *data,
> + unsigned int flags)
> +{ u32 dma_len;
> + struct scatterlist *sg = data->sg;
> +
> + if (sg->offset & 3 || sg->length & 3) {
> + dev_err(mmc_dev(host->mmc),
> + "unaligned scatterlist: os %x length %d\n",
> + sg->offset, sg->length);
> + return -EINVAL;
> + }
> +
> + dma_len = dma_map_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
> + ((data->flags & MMC_DATA_READ) ?
> + DMA_FROM_DEVICE : DMA_TO_DEVICE));
> + if (dma_len == 0) {
> + dev_err(mmc_dev(host->mmc), "dma_map_sg failed\n");
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
> +{
> + struct meson_mmc_host *host = mmc_priv(mmc);
> + struct mmc_command *cmd = mrq->cmd;
> + struct mmc_data *data = mrq->data;
> + unsigned long flags;
> + int ret;
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + if (host->error) {
> + cmd->error = host->error;
> + spin_unlock_irqrestore(&host->lock, flags);
> + mmc_request_done(mmc, mrq);
> + return;
> + }
> +
> + if (data) {
> + ret = meson_mmc_map_dma(host, data, data->flags);
> + if (ret < 0) {
> + dev_err(mmc_dev(mmc), "map DMA failed\n");
> + cmd->error = ret;
> + data->error = ret;
> + spin_unlock_irqrestore(&host->lock, flags);
> + mmc_request_done(mmc, mrq);
> + return;
> + }
> + writel(sg_dma_address(data->sg), host->base + SDIO_ADDR);
> + }
> +
> + host->mrq = mrq;
> + meson_mmc_start_cmd(mmc, mrq->cmd);
> +
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + schedule_delayed_work(&host->timeout_work, host->timeout);
> +}
> +
> +static irqreturn_t meson_mmc_irq(int irq, void *data)
> +{
> + struct meson_mmc_host *host = (void *) data;
> + struct mmc_request *mrq = host->mrq;
> + u32 irqs;
> +
> + irqs = readl(host->base + SDIO_IRQS);
> + if (mrq && (irqs & REG_IRQS_CMD_INT))
> + return IRQ_WAKE_THREAD;
> +
> + return IRQ_HANDLED;
> +}
> +
> +void meson_mmc_read_response(struct meson_mmc_host *host)
> +{
> + struct mmc_command *cmd = host->mrq->cmd;
> + u32 mult;
> + int i, resp[4] = { 0 };
> +
> + mult = readl(host->base + SDIO_MULT);
> + mult |= REG_MULT_WR_RD_OUT_IND;
> + mult &= ~(REG_MULT_RD_INDEX_M << REG_MULT_RD_INDEX_S);
> + writel(mult, host->base + SDIO_MULT);
> +
> + if (cmd->flags & MMC_RSP_136) {
> + for (i = 0; i <= 3; i++)
> + resp[3 - i] = readl(host->base + SDIO_ARGU);
> + cmd->resp[0] = (resp[0] << 8) | ((resp[1] >> 24) & 0xff);
> + cmd->resp[1] = (resp[1] << 8) | ((resp[2] >> 24) & 0xff);
> + cmd->resp[2] = (resp[2] << 8) | ((resp[3] >> 24) & 0xff);
> + cmd->resp[3] = (resp[3] << 8);
> + } else if (cmd->flags & MMC_RSP_PRESENT) {
> + cmd->resp[0] = readl(host->base + SDIO_ARGU);
> + }
> +}
> +
> +static irqreturn_t meson_mmc_irq_thread(int irq, void *irq_data)
> +{
> + struct meson_mmc_host *host = (void *) irq_data;
> + struct mmc_data *data;
> + unsigned long flags;
> + struct mmc_request *mrq;
> + u32 irqs, send;
> +
> + cancel_delayed_work_sync(&host->timeout_work);
> + spin_lock_irqsave(&host->lock, flags);
> +
> + mrq = host->mrq;
> + data = mrq->data;
> +
> + if (!mrq) {
> + spin_unlock_irqrestore(&host->lock, flags);
> + return IRQ_HANDLED;
> + }
> +
> + if (host->cmd_is_stop)
> + goto out;
> +
> + irqs = readl(host->base + SDIO_IRQS);
> + send = readl(host->base + SDIO_SEND);
> +
> + mrq->cmd->error = 0;
> +
> + if (!data) {
> + if (!((irqs & REG_IRQS_RESP_CRC7) ||
> + (send & REG_SEND_RESP_NO_CRC7)))
> + mrq->cmd->error = -EILSEQ;
> + else
> + meson_mmc_read_response(host);
> + } else {
> + if (!((irqs & REG_IRQS_RD_CRC16) ||
> + (irqs & REG_IRQS_WR_CRC16))) {
> + mrq->cmd->error = -EILSEQ;
> + } else {
> + data->bytes_xfered = data->blksz * data->blocks;
> + dma_unmap_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
> + ((data->flags & MMC_DATA_READ) ?
> + DMA_FROM_DEVICE : DMA_TO_DEVICE));
> + }
> + }
> +
> + if (mrq->stop) {
> + host->cmd_is_stop = true;
> + meson_mmc_start_cmd(host->mmc, mrq->stop);
> + spin_unlock_irqrestore(&host->lock, flags);
> + return IRQ_HANDLED;
> + }
> +
> +out:
> + host->cmd_is_stop = false;
> + host->mrq = NULL;
> + spin_unlock_irqrestore(&host->lock, flags);
> + mmc_request_done(host->mmc, mrq);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void meson_mmc_timeout(struct work_struct *work)
> +{
> + struct meson_mmc_host *host = container_of(work,
> + struct meson_mmc_host,
> + timeout_work.work);
> + struct mmc_request *mrq = host->mrq;
> + unsigned long flags;
> + u32 irqc;
> +
> + /* Do not run after meson_mmc_remove() */
> + if (host->dying)
> + return;
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + dev_err(mmc_dev(host->mmc), "Timeout on CMD%u\n", mrq->cmd->opcode);
> +
> + irqc = readl(host->base + SDIO_IRQC);
> + irqc &= ~REG_IRQC_ARC_CMD_INT;
> + writel(irqc, host->base + SDIO_IRQC);
> +
> + mrq->cmd->error = -ETIMEDOUT;
> +
> + host->mrq = NULL;
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + mmc_request_done(host->mmc, mrq);
> +}
> +
> +static struct mmc_host_ops meson_mmc_ops = {
> + .request = meson_mmc_request,
> + .set_ios = meson_mmc_set_ios,
> + .get_cd = mmc_gpio_get_cd,
> +};
> +
> +static int meson_mmc_probe(struct platform_device *pdev)
> +{
> + struct mmc_host *mmc;
> + struct meson_mmc_host *host;
> + struct pinctrl *pinctrl;
> + struct pinctrl_state *pins;
> + struct resource *res;
> + int ret, irq;
> +
> + mmc = mmc_alloc_host(sizeof(struct meson_mmc_host), &pdev->dev);
> + if (!mmc) {
> + dev_err(&pdev->dev, "mmc alloc host failed\n");
> + return -ENOMEM;
> + }
> +
> + host = mmc_priv(mmc);
> + host->mmc = mmc;
> + host->timeout = msecs_to_jiffies(2000);
I don't think this timeout will cover all cases, although I can't
really tell what a proper value should be as it's dynamically
changning between requests.
Perhaps try to pick a value that's already being used by other hosts
is good enough!?
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + host->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(host->base)) {
> + ret = PTR_ERR(host->base);
> + goto error_free_host;
> + }
> +
> + irq = platform_get_irq(pdev, 0);
> + ret = devm_request_threaded_irq(&pdev->dev, irq, meson_mmc_irq,
> + meson_mmc_irq_thread, 0, "meson_mmc",
> + host);
> + if (ret)
> + goto error_free_host;
> +
> + host->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(host->clk)) {
> + ret = PTR_ERR(host->clk);
> + goto error_free_host;
> + }
A clk_prepare_enable() is missing somwhere around here and of course
then the clock needs to be gated as well. Both in the error handling
of ->probe() but also in meson_mmc_remove().
> +
> + /* we do not support scatter lists in hardware */
> + mmc->max_segs = 1;
> + mmc->max_req_size = SDIO_BOUNCE_REQ_SIZE;
> + mmc->max_seg_size = mmc->max_req_size;
> + mmc->max_blk_count = 256;
> + mmc->max_blk_size = mmc->max_req_size / mmc->max_blk_count;
> + mmc->f_min = 300000;
> + mmc->f_max = 50000000;
> + mmc->caps |= MMC_CAP_4_BIT_DATA;
> + mmc->caps |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED;
> + mmc->caps2 |= MMC_CAP2_NO_SDIO;
> + mmc->ocr_avail = MMC_VDD_33_34;
> + mmc->ops = &meson_mmc_ops;
> +
> + spin_lock_init(&host->lock);
> +
> + INIT_DELAYED_WORK(&host->timeout_work, meson_mmc_timeout);
> +
> + pinctrl = devm_pinctrl_get(&pdev->dev);
> + if (IS_ERR(pinctrl)) {
> + ret = PTR_ERR(pinctrl);
> + goto error_free_host;
> + }
> +
> + /* We need to know at runtime which port we are using */
> + pins = pinctrl_lookup_state(pinctrl, "sdio_b");
> + if (!IS_ERR(pins))
> + host->port = 1; /* PORT_B */
> + else
> + host->port = 0; /* PORT_A */
> +
I think the pinctrl code can be removed, according to my eariler
comments for the DT documentation.
Instead you need a way to find out the currently used port...
> + ret = mmc_of_parse(mmc);
> + if (ret)
> + goto error_free_host;
> +
> + platform_set_drvdata(pdev, mmc);
> +
> + ret = mmc_add_host(mmc);
> + if (ret)
> + goto error_free_host;
> +
> + dev_info(&pdev->dev, "base:0x%p irq:%u port:%u\n",
> + host->base, irq, host->port);
> +
> + return 0;
> +
> +error_free_host:
> + mmc_free_host(mmc);
> +
> + return ret;
> +}
> +
> +static int meson_mmc_remove(struct platform_device *pdev)
> +{
> + struct mmc_host *mmc = platform_get_drvdata(pdev);
> + struct meson_mmc_host *host = mmc_priv(mmc);
> +
> + host->dying = true;
> +
> + mmc_remove_host(mmc);
> + cancel_delayed_work_sync(&host->timeout_work);
> +
> + mmc_free_host(mmc);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id meson_mmc_of_match[] = {
> + { .compatible = "amlogic,meson-mmc", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, meson_mmc_of_match);
> +
> +static struct platform_driver meson_mmc_driver = {
> + .probe = meson_mmc_probe,
> + .remove = meson_mmc_remove,
> + .driver = {
> + .name = "meson-mmc",
> + .of_match_table = of_match_ptr(meson_mmc_of_match),
> + },
> +};
> +
> +module_platform_driver(meson_mmc_driver);
> +
> +MODULE_DESCRIPTION("Meson Secure Digital Host Driver");
> +MODULE_AUTHOR("Carlo Caione <carlo@endlessm.com>");
> +MODULE_LICENSE("GPL");
> --
> 2.5.0
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
@ 2015-12-08 15:06 ` Ulf Hansson
0 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2015-12-08 15:06 UTC (permalink / raw)
To: linux-arm-kernel
On 1 December 2015 at 17:41, Carlo Caione <carlo@caione.org> wrote:
> From: Carlo Caione <carlo@endlessm.com>
>
> Add a driver for the SD/MMC host found on the Amlogic MesonX SoCs. This
> is an MMC controller which provides an interface between the application
> processor and various memory cards. It supports the SD specification
> v2.0 and the eMMC specification v4.41.
>
> This patch adds also the binding documentation.
>
> Signed-off-by: Carlo Caione <carlo@endlessm.com>
> ---
> Changelog:
>
> v2:
> * fix typo in commit message
> * avoid the bounce buffer (max_segs == 1)
>
> v3:
> * simplify meson_mmc_map_dma() and fix memory leak
>
> v4:
> * code and documentation cleanup
> * removed redundant variables
> * removed useless printing
> * introduced MMC_CAP2_NO_SDIO
> * removed state machine
> * better spinlocks management and positioning
> ---
> .../devicetree/bindings/mmc/meson-mmc.txt | 29 ++
> drivers/mmc/host/Kconfig | 7 +
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/meson-mmc.c | 527 +++++++++++++++++++++
> 4 files changed, 564 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/meson-mmc.txt
> create mode 100644 drivers/mmc/host/meson-mmc.c
>
> diff --git a/Documentation/devicetree/bindings/mmc/meson-mmc.txt b/Documentation/devicetree/bindings/mmc/meson-mmc.txt
> new file mode 100644
> index 0000000..7ff9e9e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/meson-mmc.txt
> @@ -0,0 +1,29 @@
> +* Amlogic MesonX MMC controller
> +
> +The highspeed MMC host controller on Amlogic SoCs provides an interface
> +for MMC, SD, SDIO and SDHC types of memory cards.
> +
> +Supported maximum speeds are the ones of the eMMC standard 4.41 as well
> +as the speed of SD standard 2.0.
> +
> +Required properties:
> + - compatible : "amlogic,meson-mmc"
> + - reg : mmc controller base registers
> + - interrupts : mmc controller interrupt
> + - clocks : phandle to clock provider
> + - pinctrl-names : should contain "sdio_a" or "sdio_b"
This is weird. I don't think you need this specific states.
You should be able to cope with common used "default" state.
Later in the driver code I realize that you have some register bits
that is related to either using port a or port b. But I don't think
that is the same thing as having different pinctrls states.
If you can't detect whether port a or b is used in runtime by the
driver, I suggest you add a separate meson mmc DT binding telling what
port is being used.
> + - pinctrl-0: Should specify pin control groups used for this controller
> +
> +Optional properties:
> + - for cd, bus-width and additional generic mmc parameters
> + please refer to mmc.txt within this directory
> +
> +Examples:
> + mmc0: mmc at c1108c20 {
> + pinctrl-names = "sdio_b";
> + pinctrl-0 = <&mmc0_sd_b_pins>;
> + compatible = "amlogic,meson-mmc";
> + reg = <0xc1108c20 0x20>;
> + interrupts = <0 28 1>;
> + clocks = <&clkc CLKID_CLK81>;
> + };
Overall the DT documentation looks okay (beside the comments above).
Although, please separate it to become a patch 1/2. The rest below
shoud go into a patch 2/2.
For the DT patch please include the DT maintainers when posting the new version.
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 1dee533..7f2c5ae 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -765,6 +765,13 @@ config MMC_REALTEK_USB
> Say Y here to include driver code to support SD/MMC card interface
> of Realtek RTS5129/39 series card reader
>
> +config MMC_MESON
> + tristate "Amlogic MesonX SD/MMC Host Controller support"
> + depends on ARCH_MESON
> + help
> + This selects support for the SD/MMC Host Controller on
> + Amlogic MesonX SoCs.
> +
> config MMC_SUNXI
> tristate "Allwinner sunxi SD/MMC Host Controller support"
> depends on ARCH_SUNXI
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index 3595f83..26ddf76 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -54,6 +54,7 @@ obj-$(CONFIG_MMC_VUB300) += vub300.o
> obj-$(CONFIG_MMC_USHC) += ushc.o
> obj-$(CONFIG_MMC_WMT) += wmt-sdmmc.o
> obj-$(CONFIG_MMC_MOXART) += moxart-mmc.o
> +obj-$(CONFIG_MMC_MESON) += meson-mmc.o
> obj-$(CONFIG_MMC_SUNXI) += sunxi-mmc.o
> obj-$(CONFIG_MMC_USDHI6ROL0) += usdhi6rol0.o
> obj-$(CONFIG_MMC_TOSHIBA_PCI) += toshsd.o
> diff --git a/drivers/mmc/host/meson-mmc.c b/drivers/mmc/host/meson-mmc.c
> new file mode 100644
> index 0000000..6382470
> --- /dev/null
> +++ b/drivers/mmc/host/meson-mmc.c
> @@ -0,0 +1,527 @@
> +/*
> + * meson-mmc.c - Meson SDH Controller
> + *
> + * Copyright (C) 2015 Endless Mobile, Inc.
> + * Author: Carlo Caione <carlo@endlessm.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or (at
> + * your option) any later version.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/ioport.h>
> +#include <linux/platform_device.h>
> +
> +#include <linux/mmc/host.h>
> +#include <linux/mmc/mmc.h>
> +#include <linux/mmc/sdio.h>
> +#include <linux/mmc/slot-gpio.h>
> +
> +#include <linux/of_address.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_platform.h>
> +
> +#define SDIO_ARGU (0x00)
> +#define SDIO_SEND (0x04)
> +#define SDIO_CONF (0x08)
> +#define SDIO_IRQS (0x0c)
> +#define SDIO_IRQC (0x10)
> +#define SDIO_MULT (0x14)
> +#define SDIO_ADDR (0x18)
> +#define SDIO_EXT (0x1c)
> +
> +#define REG_IRQS_RESP_CRC7 BIT(5)
> +#define REG_IRQS_RD_CRC16 BIT(6)
> +#define REG_IRQS_WR_CRC16 BIT(7)
> +#define REG_IRQS_CMD_INT BIT(9)
> +
> +#define REG_IRQC_ARC_CMD_INT BIT(4)
> +
> +#define REG_CONF_CLK_DIV_M (0x3ff)
> +#define REG_CONF_BUS_WIDTH BIT(20)
> +
> +#define REG_MULT_PORT_SEL_M (0x3)
> +#define REG_MULT_RD_INDEX_M (0x0f)
> +#define REG_MULT_RD_INDEX_S (12)
> +#define REG_MULT_WR_RD_OUT_IND BIT(8)
> +
> +#define REG_SEND_CMD_COMMAND_M (0xff)
> +#define REG_SEND_CMD_RESP_S (8)
> +#define REG_SEND_RESP_NO_CRC7 BIT(16)
> +#define REG_SEND_RESP_HAVE_DATA BIT(17)
> +#define REG_SEND_RESP_CRC7_F_8 BIT(18)
> +#define REG_SEND_CHECK_BUSY_D0 BIT(19)
> +#define REG_SEND_CMD_SEND_DATA BIT(20)
> +#define REG_SEND_REP_PACK_N_M (0xff)
> +#define REG_SEND_REP_PACK_N_S (24)
> +
> +#define REG_EXT_DAT_RW_NUM_M (0x3fff)
> +#define REG_EXT_DAT_RW_NUM_S (16)
> +
> +#define SDIO_BOUNCE_REQ_SIZE (128 * 1024)
> +
> +struct meson_mmc_host {
> + struct mmc_host *mmc;
> + struct mmc_request *mrq;
> + struct delayed_work timeout_work;
> + struct clk *clk;
> + spinlock_t lock;
> + void __iomem *base;
> + long timeout;
> + int irq;
> + int error;
> + unsigned int port;
> + bool cmd_is_stop;
> + bool dying;
> +};
> +
> +static int meson_mmc_clk_set_rate(struct mmc_host *mmc, struct mmc_ios *ios)
> +{
> + struct meson_mmc_host *host = mmc_priv(mmc);
> + unsigned long clk_rate;
> + unsigned int clk_ios = ios->clock;
> + unsigned int clk_div;
> + u32 conf_reg;
> +
> + clk_rate = clk_get_rate(host->clk);
> + if (clk_rate < 0) {
> + dev_err(mmc_dev(mmc), "cannot get clock rate\n");
> + return -EINVAL;
> + }
> +
> + clk_rate >>= 1;
> + clk_div = clk_rate / clk_ios - !(clk_rate % clk_ios);
> +
> + conf_reg = readl(host->base + SDIO_CONF);
> + conf_reg &= ~REG_CONF_CLK_DIV_M;
> + conf_reg |= clk_div;
> + writel(conf_reg, host->base + SDIO_CONF);
> +
> + dev_dbg(mmc_dev(mmc), "clk_ios: %d, clk_div: %d, clk_rate: %ld\n",
> + clk_ios, clk_div, clk_rate);
It's good practice to update mmc->actual_clock, as it's being used
from mmc core's debugfs support.
> +
> + return 0;
> +}
> +
> +static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> +{
> + struct meson_mmc_host *host = mmc_priv(mmc);
> + u32 reg;
> +
> + reg = readl(host->base + SDIO_CONF);
> +
> + switch (ios->bus_width) {
> + case MMC_BUS_WIDTH_1:
> + reg &= ~REG_CONF_BUS_WIDTH;
> + break;
> + case MMC_BUS_WIDTH_4:
> + reg |= REG_CONF_BUS_WIDTH;
> + break;
> + case MMC_BUS_WIDTH_8:
> + default:
> + dev_err(mmc_dev(mmc), "meson controller doesn't support 8bit data bus\n");
> + host->error = -EINVAL;
> + return;
> + }
> +
> + writel(reg, host->base + SDIO_CONF);
> +
> + if (ios->clock && ios->power_mode)
> + host->error = meson_mmc_clk_set_rate(mmc, ios);
> +}
> +
> +static void meson_mmc_start_cmd(struct mmc_host *mmc,
> + struct mmc_command *cmd)
> +{
> + struct meson_mmc_host *host = mmc_priv(mmc);
> + unsigned int pack_size;
> + u32 irqc, irqs, mult;
> + u32 send = 0;
> + u32 ext = 0;
> +
> + switch (mmc_resp_type(cmd)) {
> + case MMC_RSP_R1:
> + case MMC_RSP_R1B:
> + case MMC_RSP_R3:
> + send |= (45 << REG_SEND_CMD_RESP_S);
> + break;
> + case MMC_RSP_R2:
> + send |= (133 << REG_SEND_CMD_RESP_S);
> + send |= REG_SEND_RESP_CRC7_F_8;
> + break;
> + default:
> + break;
> + }
> +
> + if (!(cmd->flags & MMC_RSP_CRC))
> + send |= REG_SEND_RESP_NO_CRC7;
> +
> + if (cmd->flags & MMC_RSP_BUSY)
> + send |= REG_SEND_CHECK_BUSY_D0;
> +
> + if (cmd->data) {
> + send &= ~(REG_SEND_REP_PACK_N_M << REG_SEND_REP_PACK_N_S);
> + send |= ((cmd->data->blocks - 1) << REG_SEND_REP_PACK_N_S);
> +
> + ext &= ~(REG_EXT_DAT_RW_NUM_M << REG_EXT_DAT_RW_NUM_S);
> + if (mmc->ios.bus_width)
> + pack_size = cmd->data->blksz * 8 + (16 - 1) * 4;
> + else
> + pack_size = cmd->data->blksz * 8 + (16 - 1);
> + ext |= (pack_size << REG_EXT_DAT_RW_NUM_S);
> +
> + if (cmd->data->flags & MMC_DATA_WRITE)
> + send |= REG_SEND_CMD_SEND_DATA;
> + else
> + send |= REG_SEND_RESP_HAVE_DATA;
> + }
> +
> + send &= ~REG_SEND_CMD_COMMAND_M;
> + send |= (0x40 | cmd->opcode);
> +
> + irqc = readl(host->base + SDIO_IRQC);
> + irqc |= REG_IRQC_ARC_CMD_INT;
> +
> + irqs = readl(host->base + SDIO_IRQS);
> + irqs |= REG_IRQS_CMD_INT;
> +
> + mult = readl(host->base + SDIO_MULT);
> + mult &= ~REG_MULT_PORT_SEL_M;
> + mult |= host->port;
> + mult |= (1 << 31);
> + writel(mult, host->base + SDIO_MULT);
> + writel(irqs, host->base + SDIO_IRQS);
> + writel(irqc, host->base + SDIO_IRQC);
> +
> + writel(cmd->arg, host->base + SDIO_ARGU);
> + writel(ext, host->base + SDIO_EXT);
> + writel(send, host->base + SDIO_SEND);
> +}
> +
> +static int meson_mmc_map_dma(struct meson_mmc_host *host,
> + struct mmc_data *data,
> + unsigned int flags)
> +{ u32 dma_len;
> + struct scatterlist *sg = data->sg;
> +
> + if (sg->offset & 3 || sg->length & 3) {
> + dev_err(mmc_dev(host->mmc),
> + "unaligned scatterlist: os %x length %d\n",
> + sg->offset, sg->length);
> + return -EINVAL;
> + }
> +
> + dma_len = dma_map_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
> + ((data->flags & MMC_DATA_READ) ?
> + DMA_FROM_DEVICE : DMA_TO_DEVICE));
> + if (dma_len == 0) {
> + dev_err(mmc_dev(host->mmc), "dma_map_sg failed\n");
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
> +{
> + struct meson_mmc_host *host = mmc_priv(mmc);
> + struct mmc_command *cmd = mrq->cmd;
> + struct mmc_data *data = mrq->data;
> + unsigned long flags;
> + int ret;
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + if (host->error) {
> + cmd->error = host->error;
> + spin_unlock_irqrestore(&host->lock, flags);
> + mmc_request_done(mmc, mrq);
> + return;
> + }
> +
> + if (data) {
> + ret = meson_mmc_map_dma(host, data, data->flags);
> + if (ret < 0) {
> + dev_err(mmc_dev(mmc), "map DMA failed\n");
> + cmd->error = ret;
> + data->error = ret;
> + spin_unlock_irqrestore(&host->lock, flags);
> + mmc_request_done(mmc, mrq);
> + return;
> + }
> + writel(sg_dma_address(data->sg), host->base + SDIO_ADDR);
> + }
> +
> + host->mrq = mrq;
> + meson_mmc_start_cmd(mmc, mrq->cmd);
> +
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + schedule_delayed_work(&host->timeout_work, host->timeout);
> +}
> +
> +static irqreturn_t meson_mmc_irq(int irq, void *data)
> +{
> + struct meson_mmc_host *host = (void *) data;
> + struct mmc_request *mrq = host->mrq;
> + u32 irqs;
> +
> + irqs = readl(host->base + SDIO_IRQS);
> + if (mrq && (irqs & REG_IRQS_CMD_INT))
> + return IRQ_WAKE_THREAD;
> +
> + return IRQ_HANDLED;
> +}
> +
> +void meson_mmc_read_response(struct meson_mmc_host *host)
> +{
> + struct mmc_command *cmd = host->mrq->cmd;
> + u32 mult;
> + int i, resp[4] = { 0 };
> +
> + mult = readl(host->base + SDIO_MULT);
> + mult |= REG_MULT_WR_RD_OUT_IND;
> + mult &= ~(REG_MULT_RD_INDEX_M << REG_MULT_RD_INDEX_S);
> + writel(mult, host->base + SDIO_MULT);
> +
> + if (cmd->flags & MMC_RSP_136) {
> + for (i = 0; i <= 3; i++)
> + resp[3 - i] = readl(host->base + SDIO_ARGU);
> + cmd->resp[0] = (resp[0] << 8) | ((resp[1] >> 24) & 0xff);
> + cmd->resp[1] = (resp[1] << 8) | ((resp[2] >> 24) & 0xff);
> + cmd->resp[2] = (resp[2] << 8) | ((resp[3] >> 24) & 0xff);
> + cmd->resp[3] = (resp[3] << 8);
> + } else if (cmd->flags & MMC_RSP_PRESENT) {
> + cmd->resp[0] = readl(host->base + SDIO_ARGU);
> + }
> +}
> +
> +static irqreturn_t meson_mmc_irq_thread(int irq, void *irq_data)
> +{
> + struct meson_mmc_host *host = (void *) irq_data;
> + struct mmc_data *data;
> + unsigned long flags;
> + struct mmc_request *mrq;
> + u32 irqs, send;
> +
> + cancel_delayed_work_sync(&host->timeout_work);
> + spin_lock_irqsave(&host->lock, flags);
> +
> + mrq = host->mrq;
> + data = mrq->data;
> +
> + if (!mrq) {
> + spin_unlock_irqrestore(&host->lock, flags);
> + return IRQ_HANDLED;
> + }
> +
> + if (host->cmd_is_stop)
> + goto out;
> +
> + irqs = readl(host->base + SDIO_IRQS);
> + send = readl(host->base + SDIO_SEND);
> +
> + mrq->cmd->error = 0;
> +
> + if (!data) {
> + if (!((irqs & REG_IRQS_RESP_CRC7) ||
> + (send & REG_SEND_RESP_NO_CRC7)))
> + mrq->cmd->error = -EILSEQ;
> + else
> + meson_mmc_read_response(host);
> + } else {
> + if (!((irqs & REG_IRQS_RD_CRC16) ||
> + (irqs & REG_IRQS_WR_CRC16))) {
> + mrq->cmd->error = -EILSEQ;
> + } else {
> + data->bytes_xfered = data->blksz * data->blocks;
> + dma_unmap_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
> + ((data->flags & MMC_DATA_READ) ?
> + DMA_FROM_DEVICE : DMA_TO_DEVICE));
> + }
> + }
> +
> + if (mrq->stop) {
> + host->cmd_is_stop = true;
> + meson_mmc_start_cmd(host->mmc, mrq->stop);
> + spin_unlock_irqrestore(&host->lock, flags);
> + return IRQ_HANDLED;
> + }
> +
> +out:
> + host->cmd_is_stop = false;
> + host->mrq = NULL;
> + spin_unlock_irqrestore(&host->lock, flags);
> + mmc_request_done(host->mmc, mrq);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void meson_mmc_timeout(struct work_struct *work)
> +{
> + struct meson_mmc_host *host = container_of(work,
> + struct meson_mmc_host,
> + timeout_work.work);
> + struct mmc_request *mrq = host->mrq;
> + unsigned long flags;
> + u32 irqc;
> +
> + /* Do not run after meson_mmc_remove() */
> + if (host->dying)
> + return;
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + dev_err(mmc_dev(host->mmc), "Timeout on CMD%u\n", mrq->cmd->opcode);
> +
> + irqc = readl(host->base + SDIO_IRQC);
> + irqc &= ~REG_IRQC_ARC_CMD_INT;
> + writel(irqc, host->base + SDIO_IRQC);
> +
> + mrq->cmd->error = -ETIMEDOUT;
> +
> + host->mrq = NULL;
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + mmc_request_done(host->mmc, mrq);
> +}
> +
> +static struct mmc_host_ops meson_mmc_ops = {
> + .request = meson_mmc_request,
> + .set_ios = meson_mmc_set_ios,
> + .get_cd = mmc_gpio_get_cd,
> +};
> +
> +static int meson_mmc_probe(struct platform_device *pdev)
> +{
> + struct mmc_host *mmc;
> + struct meson_mmc_host *host;
> + struct pinctrl *pinctrl;
> + struct pinctrl_state *pins;
> + struct resource *res;
> + int ret, irq;
> +
> + mmc = mmc_alloc_host(sizeof(struct meson_mmc_host), &pdev->dev);
> + if (!mmc) {
> + dev_err(&pdev->dev, "mmc alloc host failed\n");
> + return -ENOMEM;
> + }
> +
> + host = mmc_priv(mmc);
> + host->mmc = mmc;
> + host->timeout = msecs_to_jiffies(2000);
I don't think this timeout will cover all cases, although I can't
really tell what a proper value should be as it's dynamically
changning between requests.
Perhaps try to pick a value that's already being used by other hosts
is good enough!?
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + host->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(host->base)) {
> + ret = PTR_ERR(host->base);
> + goto error_free_host;
> + }
> +
> + irq = platform_get_irq(pdev, 0);
> + ret = devm_request_threaded_irq(&pdev->dev, irq, meson_mmc_irq,
> + meson_mmc_irq_thread, 0, "meson_mmc",
> + host);
> + if (ret)
> + goto error_free_host;
> +
> + host->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(host->clk)) {
> + ret = PTR_ERR(host->clk);
> + goto error_free_host;
> + }
A clk_prepare_enable() is missing somwhere around here and of course
then the clock needs to be gated as well. Both in the error handling
of ->probe() but also in meson_mmc_remove().
> +
> + /* we do not support scatter lists in hardware */
> + mmc->max_segs = 1;
> + mmc->max_req_size = SDIO_BOUNCE_REQ_SIZE;
> + mmc->max_seg_size = mmc->max_req_size;
> + mmc->max_blk_count = 256;
> + mmc->max_blk_size = mmc->max_req_size / mmc->max_blk_count;
> + mmc->f_min = 300000;
> + mmc->f_max = 50000000;
> + mmc->caps |= MMC_CAP_4_BIT_DATA;
> + mmc->caps |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED;
> + mmc->caps2 |= MMC_CAP2_NO_SDIO;
> + mmc->ocr_avail = MMC_VDD_33_34;
> + mmc->ops = &meson_mmc_ops;
> +
> + spin_lock_init(&host->lock);
> +
> + INIT_DELAYED_WORK(&host->timeout_work, meson_mmc_timeout);
> +
> + pinctrl = devm_pinctrl_get(&pdev->dev);
> + if (IS_ERR(pinctrl)) {
> + ret = PTR_ERR(pinctrl);
> + goto error_free_host;
> + }
> +
> + /* We need to know at runtime which port we are using */
> + pins = pinctrl_lookup_state(pinctrl, "sdio_b");
> + if (!IS_ERR(pins))
> + host->port = 1; /* PORT_B */
> + else
> + host->port = 0; /* PORT_A */
> +
I think the pinctrl code can be removed, according to my eariler
comments for the DT documentation.
Instead you need a way to find out the currently used port...
> + ret = mmc_of_parse(mmc);
> + if (ret)
> + goto error_free_host;
> +
> + platform_set_drvdata(pdev, mmc);
> +
> + ret = mmc_add_host(mmc);
> + if (ret)
> + goto error_free_host;
> +
> + dev_info(&pdev->dev, "base:0x%p irq:%u port:%u\n",
> + host->base, irq, host->port);
> +
> + return 0;
> +
> +error_free_host:
> + mmc_free_host(mmc);
> +
> + return ret;
> +}
> +
> +static int meson_mmc_remove(struct platform_device *pdev)
> +{
> + struct mmc_host *mmc = platform_get_drvdata(pdev);
> + struct meson_mmc_host *host = mmc_priv(mmc);
> +
> + host->dying = true;
> +
> + mmc_remove_host(mmc);
> + cancel_delayed_work_sync(&host->timeout_work);
> +
> + mmc_free_host(mmc);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id meson_mmc_of_match[] = {
> + { .compatible = "amlogic,meson-mmc", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, meson_mmc_of_match);
> +
> +static struct platform_driver meson_mmc_driver = {
> + .probe = meson_mmc_probe,
> + .remove = meson_mmc_remove,
> + .driver = {
> + .name = "meson-mmc",
> + .of_match_table = of_match_ptr(meson_mmc_of_match),
> + },
> +};
> +
> +module_platform_driver(meson_mmc_driver);
> +
> +MODULE_DESCRIPTION("Meson Secure Digital Host Driver");
> +MODULE_AUTHOR("Carlo Caione <carlo@endlessm.com>");
> +MODULE_LICENSE("GPL");
> --
> 2.5.0
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
2015-12-08 15:06 ` Ulf Hansson
@ 2015-12-08 16:22 ` Carlo Caione
-1 siblings, 0 replies; 22+ messages in thread
From: Carlo Caione @ 2015-12-08 16:22 UTC (permalink / raw)
To: Ulf Hansson
Cc: Victor Wan, linux-meson, linux-mmc, Daniel Drake, Carlo Caione,
Carlo Caione, Jerry Cao, linux-arm-kernel
On Tue, Dec 8, 2015 at 4:06 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 1 December 2015 at 17:41, Carlo Caione <carlo@caione.org> wrote:
>> +Required properties:
>> + - compatible : "amlogic,meson-mmc"
>> + - reg : mmc controller base registers
>> + - interrupts : mmc controller interrupt
>> + - clocks : phandle to clock provider
>> + - pinctrl-names : should contain "sdio_a" or "sdio_b"
>
> This is weird. I don't think you need this specific states.
> You should be able to cope with common used "default" state.
>
> Later in the driver code I realize that you have some register bits
> that is related to either using port a or port b. But I don't think
> that is the same thing as having different pinctrls states.
Unfortunately documentation is not really clear about this. A
multi-slot capable device maybe?
> If you can't detect whether port a or b is used in runtime by the
> driver, I suggest you add a separate meson mmc DT binding telling what
> port is being used.
That was my initial idea. In v3 I had the "meson,sdio-port" property
for that. I'll put it back in the next revision.
>> + - pinctrl-0: Should specify pin control groups used for this controller
>> +
>> +Optional properties:
>> + - for cd, bus-width and additional generic mmc parameters
>> + please refer to mmc.txt within this directory
>> +
>> +Examples:
>> + mmc0: mmc@c1108c20 {
>> + pinctrl-names = "sdio_b";
>> + pinctrl-0 = <&mmc0_sd_b_pins>;
>> + compatible = "amlogic,meson-mmc";
>> + reg = <0xc1108c20 0x20>;
>> + interrupts = <0 28 1>;
>> + clocks = <&clkc CLKID_CLK81>;
>> + };
>
> Overall the DT documentation looks okay (beside the comments above).
> Although, please separate it to become a patch 1/2. The rest below
> shoud go into a patch 2/2.
>
> For the DT patch please include the DT maintainers when posting the new version.
I'll do.
[...]
>> + clk_rate >>= 1;
>> + clk_div = clk_rate / clk_ios - !(clk_rate % clk_ios);
>> +
>> + conf_reg = readl(host->base + SDIO_CONF);
>> + conf_reg &= ~REG_CONF_CLK_DIV_M;
>> + conf_reg |= clk_div;
>> + writel(conf_reg, host->base + SDIO_CONF);
>> +
>> + dev_dbg(mmc_dev(mmc), "clk_ios: %d, clk_div: %d, clk_rate: %ld\n",
>> + clk_ios, clk_div, clk_rate);
>
> It's good practice to update mmc->actual_clock, as it's being used
> from mmc core's debugfs support.
Ok
[...]
>> +
>> + host = mmc_priv(mmc);
>> + host->mmc = mmc;
>> + host->timeout = msecs_to_jiffies(2000);
>
> I don't think this timeout will cover all cases, although I can't
> really tell what a proper value should be as it's dynamically
> changning between requests.
>
> Perhaps try to pick a value that's already being used by other hosts
> is good enough!?
Other drivers use 10s, so I'll try to pick the same value.
I have actually a problem and a question related to this value. On the
boards I'm currently using to test this driver I have no CD GPIO. In
this case do I have to specify the MMC as "non-removable"? Also if I
try to change an SD card at runtime I have to wait 3 times the timeout
before being able to do it successfully.
[...]
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + host->base = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(host->base)) {
>> + ret = PTR_ERR(host->base);
>> + goto error_free_host;
>> + }
>> +
>> + irq = platform_get_irq(pdev, 0);
>> + ret = devm_request_threaded_irq(&pdev->dev, irq, meson_mmc_irq,
>> + meson_mmc_irq_thread, 0, "meson_mmc",
>> + host);
>> + if (ret)
>> + goto error_free_host;
>> +
>> + host->clk = devm_clk_get(&pdev->dev, NULL);
>> + if (IS_ERR(host->clk)) {
>> + ret = PTR_ERR(host->clk);
>> + goto error_free_host;
>> + }
>
> A clk_prepare_enable() is missing somwhere around here and of course
> then the clock needs to be gated as well. Both in the error handling
> of ->probe() but also in meson_mmc_remove().
No clock gatings yet on this platform but I'll fix it.
>> +
>> + /* we do not support scatter lists in hardware */
>> + mmc->max_segs = 1;
>> + mmc->max_req_size = SDIO_BOUNCE_REQ_SIZE;
>> + mmc->max_seg_size = mmc->max_req_size;
>> + mmc->max_blk_count = 256;
>> + mmc->max_blk_size = mmc->max_req_size / mmc->max_blk_count;
>> + mmc->f_min = 300000;
>> + mmc->f_max = 50000000;
>> + mmc->caps |= MMC_CAP_4_BIT_DATA;
>> + mmc->caps |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED;
>> + mmc->caps2 |= MMC_CAP2_NO_SDIO;
>> + mmc->ocr_avail = MMC_VDD_33_34;
>> + mmc->ops = &meson_mmc_ops;
>> +
>> + spin_lock_init(&host->lock);
>> +
>> + INIT_DELAYED_WORK(&host->timeout_work, meson_mmc_timeout);
>> +
>> + pinctrl = devm_pinctrl_get(&pdev->dev);
>> + if (IS_ERR(pinctrl)) {
>> + ret = PTR_ERR(pinctrl);
>> + goto error_free_host;
>> + }
>> +
>> + /* We need to know at runtime which port we are using */
>> + pins = pinctrl_lookup_state(pinctrl, "sdio_b");
>> + if (!IS_ERR(pins))
>> + host->port = 1; /* PORT_B */
>> + else
>> + host->port = 0; /* PORT_A */
>> +
>
> I think the pinctrl code can be removed, according to my eariler
> comments for the DT documentation.
Agree
> Instead you need a way to find out the currently used port...
I really do not think this is feasible from the driver at runtime. So
I'll go with the new DT property.
[...]
Thanks,
--
Carlo Caione
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
@ 2015-12-08 16:22 ` Carlo Caione
0 siblings, 0 replies; 22+ messages in thread
From: Carlo Caione @ 2015-12-08 16:22 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Dec 8, 2015 at 4:06 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 1 December 2015 at 17:41, Carlo Caione <carlo@caione.org> wrote:
>> +Required properties:
>> + - compatible : "amlogic,meson-mmc"
>> + - reg : mmc controller base registers
>> + - interrupts : mmc controller interrupt
>> + - clocks : phandle to clock provider
>> + - pinctrl-names : should contain "sdio_a" or "sdio_b"
>
> This is weird. I don't think you need this specific states.
> You should be able to cope with common used "default" state.
>
> Later in the driver code I realize that you have some register bits
> that is related to either using port a or port b. But I don't think
> that is the same thing as having different pinctrls states.
Unfortunately documentation is not really clear about this. A
multi-slot capable device maybe?
> If you can't detect whether port a or b is used in runtime by the
> driver, I suggest you add a separate meson mmc DT binding telling what
> port is being used.
That was my initial idea. In v3 I had the "meson,sdio-port" property
for that. I'll put it back in the next revision.
>> + - pinctrl-0: Should specify pin control groups used for this controller
>> +
>> +Optional properties:
>> + - for cd, bus-width and additional generic mmc parameters
>> + please refer to mmc.txt within this directory
>> +
>> +Examples:
>> + mmc0: mmc at c1108c20 {
>> + pinctrl-names = "sdio_b";
>> + pinctrl-0 = <&mmc0_sd_b_pins>;
>> + compatible = "amlogic,meson-mmc";
>> + reg = <0xc1108c20 0x20>;
>> + interrupts = <0 28 1>;
>> + clocks = <&clkc CLKID_CLK81>;
>> + };
>
> Overall the DT documentation looks okay (beside the comments above).
> Although, please separate it to become a patch 1/2. The rest below
> shoud go into a patch 2/2.
>
> For the DT patch please include the DT maintainers when posting the new version.
I'll do.
[...]
>> + clk_rate >>= 1;
>> + clk_div = clk_rate / clk_ios - !(clk_rate % clk_ios);
>> +
>> + conf_reg = readl(host->base + SDIO_CONF);
>> + conf_reg &= ~REG_CONF_CLK_DIV_M;
>> + conf_reg |= clk_div;
>> + writel(conf_reg, host->base + SDIO_CONF);
>> +
>> + dev_dbg(mmc_dev(mmc), "clk_ios: %d, clk_div: %d, clk_rate: %ld\n",
>> + clk_ios, clk_div, clk_rate);
>
> It's good practice to update mmc->actual_clock, as it's being used
> from mmc core's debugfs support.
Ok
[...]
>> +
>> + host = mmc_priv(mmc);
>> + host->mmc = mmc;
>> + host->timeout = msecs_to_jiffies(2000);
>
> I don't think this timeout will cover all cases, although I can't
> really tell what a proper value should be as it's dynamically
> changning between requests.
>
> Perhaps try to pick a value that's already being used by other hosts
> is good enough!?
Other drivers use 10s, so I'll try to pick the same value.
I have actually a problem and a question related to this value. On the
boards I'm currently using to test this driver I have no CD GPIO. In
this case do I have to specify the MMC as "non-removable"? Also if I
try to change an SD card at runtime I have to wait 3 times the timeout
before being able to do it successfully.
[...]
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + host->base = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(host->base)) {
>> + ret = PTR_ERR(host->base);
>> + goto error_free_host;
>> + }
>> +
>> + irq = platform_get_irq(pdev, 0);
>> + ret = devm_request_threaded_irq(&pdev->dev, irq, meson_mmc_irq,
>> + meson_mmc_irq_thread, 0, "meson_mmc",
>> + host);
>> + if (ret)
>> + goto error_free_host;
>> +
>> + host->clk = devm_clk_get(&pdev->dev, NULL);
>> + if (IS_ERR(host->clk)) {
>> + ret = PTR_ERR(host->clk);
>> + goto error_free_host;
>> + }
>
> A clk_prepare_enable() is missing somwhere around here and of course
> then the clock needs to be gated as well. Both in the error handling
> of ->probe() but also in meson_mmc_remove().
No clock gatings yet on this platform but I'll fix it.
>> +
>> + /* we do not support scatter lists in hardware */
>> + mmc->max_segs = 1;
>> + mmc->max_req_size = SDIO_BOUNCE_REQ_SIZE;
>> + mmc->max_seg_size = mmc->max_req_size;
>> + mmc->max_blk_count = 256;
>> + mmc->max_blk_size = mmc->max_req_size / mmc->max_blk_count;
>> + mmc->f_min = 300000;
>> + mmc->f_max = 50000000;
>> + mmc->caps |= MMC_CAP_4_BIT_DATA;
>> + mmc->caps |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED;
>> + mmc->caps2 |= MMC_CAP2_NO_SDIO;
>> + mmc->ocr_avail = MMC_VDD_33_34;
>> + mmc->ops = &meson_mmc_ops;
>> +
>> + spin_lock_init(&host->lock);
>> +
>> + INIT_DELAYED_WORK(&host->timeout_work, meson_mmc_timeout);
>> +
>> + pinctrl = devm_pinctrl_get(&pdev->dev);
>> + if (IS_ERR(pinctrl)) {
>> + ret = PTR_ERR(pinctrl);
>> + goto error_free_host;
>> + }
>> +
>> + /* We need to know at runtime which port we are using */
>> + pins = pinctrl_lookup_state(pinctrl, "sdio_b");
>> + if (!IS_ERR(pins))
>> + host->port = 1; /* PORT_B */
>> + else
>> + host->port = 0; /* PORT_A */
>> +
>
> I think the pinctrl code can be removed, according to my eariler
> comments for the DT documentation.
Agree
> Instead you need a way to find out the currently used port...
I really do not think this is feasible from the driver at runtime. So
I'll go with the new DT property.
[...]
Thanks,
--
Carlo Caione
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
2015-12-08 16:22 ` Carlo Caione
@ 2015-12-08 20:35 ` Ulf Hansson
-1 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2015-12-08 20:35 UTC (permalink / raw)
To: Carlo Caione
Cc: linux-arm-kernel, linux-mmc, Daniel Drake, Jerry Cao, Victor Wan,
linux-meson, Carlo Caione
[...]
>
> I have actually a problem and a question related to this value. On the
> boards I'm currently using to test this driver I have no CD GPIO. In
> this case do I have to specify the MMC as "non-removable"? Also if I
"non-removable" is intended to be used for eMMC or other cards
(SD/SDIO) that is not possible to remove/insert at runtime.
> try to change an SD card at runtime I have to wait 3 times the timeout
> before being able to do it successfully.
I guess you are using the "broken-cd" binding, which will enable
MMC_CAP_NEEDS_POLL and thus the mmc core will poll to detect cards
being inserted or removed.
[...]
Kind regards
Uffe
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
@ 2015-12-08 20:35 ` Ulf Hansson
0 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2015-12-08 20:35 UTC (permalink / raw)
To: linux-arm-kernel
[...]
>
> I have actually a problem and a question related to this value. On the
> boards I'm currently using to test this driver I have no CD GPIO. In
> this case do I have to specify the MMC as "non-removable"? Also if I
"non-removable" is intended to be used for eMMC or other cards
(SD/SDIO) that is not possible to remove/insert at runtime.
> try to change an SD card at runtime I have to wait 3 times the timeout
> before being able to do it successfully.
I guess you are using the "broken-cd" binding, which will enable
MMC_CAP_NEEDS_POLL and thus the mmc core will poll to detect cards
being inserted or removed.
[...]
Kind regards
Uffe
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
2015-12-08 20:35 ` Ulf Hansson
@ 2016-02-01 22:05 ` Carlo Caione
-1 siblings, 0 replies; 22+ messages in thread
From: Carlo Caione @ 2016-02-01 22:05 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-arm-kernel, linux-mmc, linux-meson, linux
On Tue, Dec 8, 2015 at 9:35 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
Hi Ulf,
(old thread, I know)
>> I have actually a problem and a question related to this value. On the
>> boards I'm currently using to test this driver I have no CD GPIO. In
>> this case do I have to specify the MMC as "non-removable"? Also if I
>
> "non-removable" is intended to be used for eMMC or other cards
> (SD/SDIO) that is not possible to remove/insert at runtime.
>
>> try to change an SD card at runtime I have to wait 3 times the timeout
>> before being able to do it successfully.
>
> I guess you are using the "broken-cd" binding, which will enable
> MMC_CAP_NEEDS_POLL and thus the mmc core will poll to detect cards
> being inserted or removed.
I still fail to see what the correct behaviour should be.
Using the v4 of this driver with a "broken-cd" binding and a timeout
of 10s my SD card takes 30s to be identified as removed after I take
it out from the SD slot.
Also, if I take it out and insert it again, I still have to wait 30s
to be able to access it again. Is this a limitation / bug of my driver
or is this the expected behaviour when we do not have a dedicated GPIO
for card detection?
Cheers,
--
Carlo Caione
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
@ 2016-02-01 22:05 ` Carlo Caione
0 siblings, 0 replies; 22+ messages in thread
From: Carlo Caione @ 2016-02-01 22:05 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Dec 8, 2015 at 9:35 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
Hi Ulf,
(old thread, I know)
>> I have actually a problem and a question related to this value. On the
>> boards I'm currently using to test this driver I have no CD GPIO. In
>> this case do I have to specify the MMC as "non-removable"? Also if I
>
> "non-removable" is intended to be used for eMMC or other cards
> (SD/SDIO) that is not possible to remove/insert at runtime.
>
>> try to change an SD card at runtime I have to wait 3 times the timeout
>> before being able to do it successfully.
>
> I guess you are using the "broken-cd" binding, which will enable
> MMC_CAP_NEEDS_POLL and thus the mmc core will poll to detect cards
> being inserted or removed.
I still fail to see what the correct behaviour should be.
Using the v4 of this driver with a "broken-cd" binding and a timeout
of 10s my SD card takes 30s to be identified as removed after I take
it out from the SD slot.
Also, if I take it out and insert it again, I still have to wait 30s
to be able to access it again. Is this a limitation / bug of my driver
or is this the expected behaviour when we do not have a dedicated GPIO
for card detection?
Cheers,
--
Carlo Caione
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
2016-02-01 22:05 ` Carlo Caione
@ 2016-02-02 11:21 ` Ulf Hansson
-1 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2016-02-02 11:21 UTC (permalink / raw)
To: Carlo Caione; +Cc: linux-arm-kernel, linux-mmc, linux-meson, linux
On 1 February 2016 at 23:05, Carlo Caione <carlo@caione.org> wrote:
> On Tue, Dec 8, 2015 at 9:35 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> Hi Ulf,
> (old thread, I know)
>
>>> I have actually a problem and a question related to this value. On the
>>> boards I'm currently using to test this driver I have no CD GPIO. In
>>> this case do I have to specify the MMC as "non-removable"? Also if I
>>
>> "non-removable" is intended to be used for eMMC or other cards
>> (SD/SDIO) that is not possible to remove/insert at runtime.
>>
>>> try to change an SD card at runtime I have to wait 3 times the timeout
>>> before being able to do it successfully.
>>
>> I guess you are using the "broken-cd" binding, which will enable
>> MMC_CAP_NEEDS_POLL and thus the mmc core will poll to detect cards
>> being inserted or removed.
>
> I still fail to see what the correct behaviour should be.
> Using the v4 of this driver with a "broken-cd" binding and a timeout
> of 10s my SD card takes 30s to be identified as removed after I take
> it out from the SD slot.
> Also, if I take it out and insert it again, I still have to wait 30s
> to be able to access it again. Is this a limitation / bug of my driver
> or is this the expected behaviour when we do not have a dedicated GPIO
> for card detection?
It's *not* an expected a behaviour.
Using MMC_CAP_NEEDS_POLL, means that the mmc core will send CMD13
commands in a polling manner to find out whether a card has been
removed.
It seems like when the card is removed, the host driver doesn't get an
IRQ which indicates a command timeout. Instead it waits for the 10 s
timeout to expire.
My question is then; why don't the driver get an IRQ for the command
timeout? Is that because of non-correct setup of the IRQ masks or
because the controller HW doesn't support this?
Kind regards
Uffe
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
@ 2016-02-02 11:21 ` Ulf Hansson
0 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2016-02-02 11:21 UTC (permalink / raw)
To: linux-arm-kernel
On 1 February 2016 at 23:05, Carlo Caione <carlo@caione.org> wrote:
> On Tue, Dec 8, 2015 at 9:35 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> Hi Ulf,
> (old thread, I know)
>
>>> I have actually a problem and a question related to this value. On the
>>> boards I'm currently using to test this driver I have no CD GPIO. In
>>> this case do I have to specify the MMC as "non-removable"? Also if I
>>
>> "non-removable" is intended to be used for eMMC or other cards
>> (SD/SDIO) that is not possible to remove/insert at runtime.
>>
>>> try to change an SD card at runtime I have to wait 3 times the timeout
>>> before being able to do it successfully.
>>
>> I guess you are using the "broken-cd" binding, which will enable
>> MMC_CAP_NEEDS_POLL and thus the mmc core will poll to detect cards
>> being inserted or removed.
>
> I still fail to see what the correct behaviour should be.
> Using the v4 of this driver with a "broken-cd" binding and a timeout
> of 10s my SD card takes 30s to be identified as removed after I take
> it out from the SD slot.
> Also, if I take it out and insert it again, I still have to wait 30s
> to be able to access it again. Is this a limitation / bug of my driver
> or is this the expected behaviour when we do not have a dedicated GPIO
> for card detection?
It's *not* an expected a behaviour.
Using MMC_CAP_NEEDS_POLL, means that the mmc core will send CMD13
commands in a polling manner to find out whether a card has been
removed.
It seems like when the card is removed, the host driver doesn't get an
IRQ which indicates a command timeout. Instead it waits for the 10 s
timeout to expire.
My question is then; why don't the driver get an IRQ for the command
timeout? Is that because of non-correct setup of the IRQ masks or
because the controller HW doesn't support this?
Kind regards
Uffe
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
2016-02-02 11:21 ` Ulf Hansson
@ 2016-02-02 21:54 ` Carlo Caione
-1 siblings, 0 replies; 22+ messages in thread
From: Carlo Caione @ 2016-02-02 21:54 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Carlo Caione, linux-arm-kernel, linux-mmc, linux-meson, linux
On Tue, Feb 2, 2016 at 12:21 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 1 February 2016 at 23:05, Carlo Caione <carlo@caione.org> wrote:
>> On Tue, Dec 8, 2015 at 9:35 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
[cut]
>> I still fail to see what the correct behaviour should be.
>> Using the v4 of this driver with a "broken-cd" binding and a timeout
>> of 10s my SD card takes 30s to be identified as removed after I take
>> it out from the SD slot.
>> Also, if I take it out and insert it again, I still have to wait 30s
>> to be able to access it again. Is this a limitation / bug of my driver
>> or is this the expected behaviour when we do not have a dedicated GPIO
>> for card detection?
>
> It's *not* an expected a behaviour.
>
> Using MMC_CAP_NEEDS_POLL, means that the mmc core will send CMD13
> commands in a polling manner to find out whether a card has been
> removed.
>
> It seems like when the card is removed, the host driver doesn't get an
> IRQ which indicates a command timeout. Instead it waits for the 10 s
> timeout to expire.
>
> My question is then; why don't the driver get an IRQ for the command
> timeout? Is that because of non-correct setup of the IRQ masks or
> because the controller HW doesn't support this?
I just verified that apparently the HW doesn't support this (no
register for commands timeout and no IRQ generated).
Anything wrong with implementing this in software with a timer on
mrq->data->timeout_ns?
--
Carlo Caione
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
@ 2016-02-02 21:54 ` Carlo Caione
0 siblings, 0 replies; 22+ messages in thread
From: Carlo Caione @ 2016-02-02 21:54 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Feb 2, 2016 at 12:21 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 1 February 2016 at 23:05, Carlo Caione <carlo@caione.org> wrote:
>> On Tue, Dec 8, 2015 at 9:35 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
[cut]
>> I still fail to see what the correct behaviour should be.
>> Using the v4 of this driver with a "broken-cd" binding and a timeout
>> of 10s my SD card takes 30s to be identified as removed after I take
>> it out from the SD slot.
>> Also, if I take it out and insert it again, I still have to wait 30s
>> to be able to access it again. Is this a limitation / bug of my driver
>> or is this the expected behaviour when we do not have a dedicated GPIO
>> for card detection?
>
> It's *not* an expected a behaviour.
>
> Using MMC_CAP_NEEDS_POLL, means that the mmc core will send CMD13
> commands in a polling manner to find out whether a card has been
> removed.
>
> It seems like when the card is removed, the host driver doesn't get an
> IRQ which indicates a command timeout. Instead it waits for the 10 s
> timeout to expire.
>
> My question is then; why don't the driver get an IRQ for the command
> timeout? Is that because of non-correct setup of the IRQ masks or
> because the controller HW doesn't support this?
I just verified that apparently the HW doesn't support this (no
register for commands timeout and no IRQ generated).
Anything wrong with implementing this in software with a timer on
mrq->data->timeout_ns?
--
Carlo Caione
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
2016-02-02 21:54 ` Carlo Caione
@ 2016-02-03 10:46 ` Ulf Hansson
-1 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2016-02-03 10:46 UTC (permalink / raw)
To: Carlo Caione; +Cc: linux-arm-kernel, linux-mmc, linux-meson, linux
On 2 February 2016 at 22:54, Carlo Caione <carlo@caione.org> wrote:
> On Tue, Feb 2, 2016 at 12:21 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 1 February 2016 at 23:05, Carlo Caione <carlo@caione.org> wrote:
>>> On Tue, Dec 8, 2015 at 9:35 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> [cut]
>
>>> I still fail to see what the correct behaviour should be.
>>> Using the v4 of this driver with a "broken-cd" binding and a timeout
>>> of 10s my SD card takes 30s to be identified as removed after I take
>>> it out from the SD slot.
>>> Also, if I take it out and insert it again, I still have to wait 30s
>>> to be able to access it again. Is this a limitation / bug of my driver
>>> or is this the expected behaviour when we do not have a dedicated GPIO
>>> for card detection?
>>
>> It's *not* an expected a behaviour.
>>
>> Using MMC_CAP_NEEDS_POLL, means that the mmc core will send CMD13
>> commands in a polling manner to find out whether a card has been
>> removed.
>>
>> It seems like when the card is removed, the host driver doesn't get an
>> IRQ which indicates a command timeout. Instead it waits for the 10 s
>> timeout to expire.
>>
>> My question is then; why don't the driver get an IRQ for the command
>> timeout? Is that because of non-correct setup of the IRQ masks or
>> because the controller HW doesn't support this?
>
> I just verified that apparently the HW doesn't support this (no
> register for commands timeout and no IRQ generated).
> Anything wrong with implementing this in software with a timer on
> mrq->data->timeout_ns?
Huh, another broken MMC controller. :-)
In some cases there are no data in the request, it's just a command
being sent. So to deal with this issue, you will have to adjust the
timer depending on the request we send to the controller.
Is there anyway you can find out if there is a card inserted? I
understand you didn't have GPIO, but perhaps you have some internal
mechanism in the controller to for example poll a register to know
this?
Kind regards
Uffe
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
@ 2016-02-03 10:46 ` Ulf Hansson
0 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2016-02-03 10:46 UTC (permalink / raw)
To: linux-arm-kernel
On 2 February 2016 at 22:54, Carlo Caione <carlo@caione.org> wrote:
> On Tue, Feb 2, 2016 at 12:21 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 1 February 2016 at 23:05, Carlo Caione <carlo@caione.org> wrote:
>>> On Tue, Dec 8, 2015 at 9:35 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> [cut]
>
>>> I still fail to see what the correct behaviour should be.
>>> Using the v4 of this driver with a "broken-cd" binding and a timeout
>>> of 10s my SD card takes 30s to be identified as removed after I take
>>> it out from the SD slot.
>>> Also, if I take it out and insert it again, I still have to wait 30s
>>> to be able to access it again. Is this a limitation / bug of my driver
>>> or is this the expected behaviour when we do not have a dedicated GPIO
>>> for card detection?
>>
>> It's *not* an expected a behaviour.
>>
>> Using MMC_CAP_NEEDS_POLL, means that the mmc core will send CMD13
>> commands in a polling manner to find out whether a card has been
>> removed.
>>
>> It seems like when the card is removed, the host driver doesn't get an
>> IRQ which indicates a command timeout. Instead it waits for the 10 s
>> timeout to expire.
>>
>> My question is then; why don't the driver get an IRQ for the command
>> timeout? Is that because of non-correct setup of the IRQ masks or
>> because the controller HW doesn't support this?
>
> I just verified that apparently the HW doesn't support this (no
> register for commands timeout and no IRQ generated).
> Anything wrong with implementing this in software with a timer on
> mrq->data->timeout_ns?
Huh, another broken MMC controller. :-)
In some cases there are no data in the request, it's just a command
being sent. So to deal with this issue, you will have to adjust the
timer depending on the request we send to the controller.
Is there anyway you can find out if there is a card inserted? I
understand you didn't have GPIO, but perhaps you have some internal
mechanism in the controller to for example poll a register to know
this?
Kind regards
Uffe
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
2015-12-01 16:41 ` Carlo Caione
@ 2016-02-03 12:17 ` Stefan Wahren
-1 siblings, 0 replies; 22+ messages in thread
From: Stefan Wahren @ 2016-02-03 12:17 UTC (permalink / raw)
To: Carlo Caione
Cc: linux-arm-kernel, linux-mmc, drake, jerry.cao, victor.wan,
linux-meson, ulf.hansson, Carlo Caione
Hi Carlo,
Am 01.12.2015 um 17:41 schrieb Carlo Caione:
> From: Carlo Caione <carlo@endlessm.com>
>
> Add a driver for the SD/MMC host found on the Amlogic MesonX SoCs. This
> is an MMC controller which provides an interface between the application
> processor and various memory cards. It supports the SD specification
> v2.0 and the eMMC specification v4.41.
>
> This patch adds also the binding documentation.
according to Documentation/devicetree/bindings/submitting-patches.txt
the binding should be a separate patch in a series.
Stefan
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
@ 2016-02-03 12:17 ` Stefan Wahren
0 siblings, 0 replies; 22+ messages in thread
From: Stefan Wahren @ 2016-02-03 12:17 UTC (permalink / raw)
To: linux-arm-kernel
Hi Carlo,
Am 01.12.2015 um 17:41 schrieb Carlo Caione:
> From: Carlo Caione <carlo@endlessm.com>
>
> Add a driver for the SD/MMC host found on the Amlogic MesonX SoCs. This
> is an MMC controller which provides an interface between the application
> processor and various memory cards. It supports the SD specification
> v2.0 and the eMMC specification v4.41.
>
> This patch adds also the binding documentation.
according to Documentation/devicetree/bindings/submitting-patches.txt
the binding should be a separate patch in a series.
Stefan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
2016-02-03 10:46 ` Ulf Hansson
@ 2016-02-04 9:12 ` Carlo Caione
-1 siblings, 0 replies; 22+ messages in thread
From: Carlo Caione @ 2016-02-04 9:12 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Carlo Caione, linux-arm-kernel, linux-mmc, linux-meson, linux
On Wed, Feb 3, 2016 at 11:46 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 2 February 2016 at 22:54, Carlo Caione <carlo@caione.org> wrote:
[cut]
> Huh, another broken MMC controller. :-)
Oh, actually I found in the datasheet a couple of undocumented
registers (also not used in the original Amlogic SDK code) that seems
to have something to do with timeout (hard to say without proper
documentation but they are called "Timeout counter for preload setting
and present status" and "Timeout Counter Interrupt Enable for ARC/ARM"
so I guess I'm on the right track here).
I briefly tried yesterday to enable this timeout interrupt with a
dummy value in the counter register (I have no idea how get this value
from the clocking) and it seems working (the counter value is
decremented when reading the register upon command completion).
> In some cases there are no data in the request, it's just a command
> being sent. So to deal with this issue, you will have to adjust the
> timer depending on the request we send to the controller.
I see that the CMD13 used to poll the card has no data so in that case
should we use a fixed / dummy value to use as timeout counter?
I'm kind of confused here since the MMC drivers I'm looking at use the
hardware timeout register only when the data->timeout_ns is actually
present.
--
Carlo Caione
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
@ 2016-02-04 9:12 ` Carlo Caione
0 siblings, 0 replies; 22+ messages in thread
From: Carlo Caione @ 2016-02-04 9:12 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Feb 3, 2016 at 11:46 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 2 February 2016 at 22:54, Carlo Caione <carlo@caione.org> wrote:
[cut]
> Huh, another broken MMC controller. :-)
Oh, actually I found in the datasheet a couple of undocumented
registers (also not used in the original Amlogic SDK code) that seems
to have something to do with timeout (hard to say without proper
documentation but they are called "Timeout counter for preload setting
and present status" and "Timeout Counter Interrupt Enable for ARC/ARM"
so I guess I'm on the right track here).
I briefly tried yesterday to enable this timeout interrupt with a
dummy value in the counter register (I have no idea how get this value
from the clocking) and it seems working (the counter value is
decremented when reading the register upon command completion).
> In some cases there are no data in the request, it's just a command
> being sent. So to deal with this issue, you will have to adjust the
> timer depending on the request we send to the controller.
I see that the CMD13 used to poll the card has no data so in that case
should we use a fixed / dummy value to use as timeout counter?
I'm kind of confused here since the MMC drivers I'm looking at use the
hardware timeout register only when the data->timeout_ns is actually
present.
--
Carlo Caione
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
2016-02-04 9:12 ` Carlo Caione
@ 2016-02-04 10:34 ` Ulf Hansson
-1 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2016-02-04 10:34 UTC (permalink / raw)
To: Carlo Caione; +Cc: linux-arm-kernel, linux-mmc, linux-meson, linux
On 4 February 2016 at 10:12, Carlo Caione <carlo@caione.org> wrote:
> On Wed, Feb 3, 2016 at 11:46 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 2 February 2016 at 22:54, Carlo Caione <carlo@caione.org> wrote:
>
> [cut]
>
>> Huh, another broken MMC controller. :-)
>
> Oh, actually I found in the datasheet a couple of undocumented
> registers (also not used in the original Amlogic SDK code) that seems
> to have something to do with timeout (hard to say without proper
> documentation but they are called "Timeout counter for preload setting
> and present status" and "Timeout Counter Interrupt Enable for ARC/ARM"
> so I guess I'm on the right track here).
> I briefly tried yesterday to enable this timeout interrupt with a
> dummy value in the counter register (I have no idea how get this value
> from the clocking) and it seems working (the counter value is
> decremented when reading the register upon command completion).
Let's hope this works, as this will for sure be a better solution.
>
>> In some cases there are no data in the request, it's just a command
>> being sent. So to deal with this issue, you will have to adjust the
>> timer depending on the request we send to the controller.
>
> I see that the CMD13 used to poll the card has no data so in that case
> should we use a fixed / dummy value to use as timeout counter?
> I'm kind of confused here since the MMC drivers I'm looking at use the
> hardware timeout register only when the data->timeout_ns is actually
> present.
That's a different timeout value. It's elated to *each* block data
transfer and not the entire request. So it provides information to the
controller about what delays it shall accept during a block being
written/read.
More details is found in the electrical descriptions of the interface
in the eMMC/SD spec.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs
@ 2016-02-04 10:34 ` Ulf Hansson
0 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2016-02-04 10:34 UTC (permalink / raw)
To: linux-arm-kernel
On 4 February 2016 at 10:12, Carlo Caione <carlo@caione.org> wrote:
> On Wed, Feb 3, 2016 at 11:46 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 2 February 2016 at 22:54, Carlo Caione <carlo@caione.org> wrote:
>
> [cut]
>
>> Huh, another broken MMC controller. :-)
>
> Oh, actually I found in the datasheet a couple of undocumented
> registers (also not used in the original Amlogic SDK code) that seems
> to have something to do with timeout (hard to say without proper
> documentation but they are called "Timeout counter for preload setting
> and present status" and "Timeout Counter Interrupt Enable for ARC/ARM"
> so I guess I'm on the right track here).
> I briefly tried yesterday to enable this timeout interrupt with a
> dummy value in the counter register (I have no idea how get this value
> from the clocking) and it seems working (the counter value is
> decremented when reading the register upon command completion).
Let's hope this works, as this will for sure be a better solution.
>
>> In some cases there are no data in the request, it's just a command
>> being sent. So to deal with this issue, you will have to adjust the
>> timer depending on the request we send to the controller.
>
> I see that the CMD13 used to poll the card has no data so in that case
> should we use a fixed / dummy value to use as timeout counter?
> I'm kind of confused here since the MMC drivers I'm looking at use the
> hardware timeout register only when the data->timeout_ns is actually
> present.
That's a different timeout value. It's elated to *each* block data
transfer and not the entire request. So it provides information to the
controller about what delays it shall accept during a block being
written/read.
More details is found in the electrical descriptions of the interface
in the eMMC/SD spec.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2016-02-04 10:34 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-01 16:41 [PATCH v4] mmc: meson: Add driver for the SD/MMC host found on Amlogic MesonX SoCs Carlo Caione
2015-12-01 16:41 ` Carlo Caione
2015-12-08 15:06 ` Ulf Hansson
2015-12-08 15:06 ` Ulf Hansson
2015-12-08 16:22 ` Carlo Caione
2015-12-08 16:22 ` Carlo Caione
2015-12-08 20:35 ` Ulf Hansson
2015-12-08 20:35 ` Ulf Hansson
2016-02-01 22:05 ` Carlo Caione
2016-02-01 22:05 ` Carlo Caione
2016-02-02 11:21 ` Ulf Hansson
2016-02-02 11:21 ` Ulf Hansson
2016-02-02 21:54 ` Carlo Caione
2016-02-02 21:54 ` Carlo Caione
2016-02-03 10:46 ` Ulf Hansson
2016-02-03 10:46 ` Ulf Hansson
2016-02-04 9:12 ` Carlo Caione
2016-02-04 9:12 ` Carlo Caione
2016-02-04 10:34 ` Ulf Hansson
2016-02-04 10:34 ` Ulf Hansson
2016-02-03 12:17 ` Stefan Wahren
2016-02-03 12:17 ` Stefan Wahren
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.