* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-05 2:18 [PATCH 0/7] Add mmc driver for i.MX23/28 Shawn Guo
@ 2011-02-05 2:18 ` Shawn Guo
2011-02-04 20:26 ` Arnd Bergmann
` (2 more replies)
2011-02-05 2:18 ` [PATCH 2/7] ARM: mxs/clock: fix base address missing in name##_set_parent Shawn Guo
` (5 subsequent siblings)
6 siblings, 3 replies; 33+ messages in thread
From: Shawn Guo @ 2011-02-05 2:18 UTC (permalink / raw)
To: linux-arm-kernel
This adds the mmc host driver for Freescale MXS-based SoC i.MX23/28.
The driver calls into mxs-dma via generic dmaengine api for both pio
and data transfer.
Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
---
arch/arm/mach-mxs/include/mach/mmc.h | 15 +
drivers/mmc/host/Kconfig | 9 +
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/mxs-mmc.c | 884 ++++++++++++++++++++++++++++++++++
4 files changed, 909 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-mxs/include/mach/mmc.h
create mode 100644 drivers/mmc/host/mxs-mmc.c
diff --git a/arch/arm/mach-mxs/include/mach/mmc.h b/arch/arm/mach-mxs/include/mach/mmc.h
new file mode 100644
index 0000000..42a1842
--- /dev/null
+++ b/arch/arm/mach-mxs/include/mach/mmc.h
@@ -0,0 +1,15 @@
+/*
+ * Copyright 2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __MACH_MXS_MMC_H__
+#define __MACH_MXS_MMC_H__
+
+struct mxs_mmc_platform_data {
+ int wp_gpio; /* write protect pin */
+};
+#endif /* __MACH_MXS_MMC_H__ */
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index afe8c6f..42a9e21 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -319,6 +319,15 @@ config MMC_MXC
If unsure, say N.
+config MMC_MXS
+ tristate "Freescale MXS Multimedia Card Interface support"
+ depends on ARCH_MXS
+ help
+ This selects the Freescale SSP MMC controller found on MXS based
+ platforms like mx23/28.
+
+ If unsure, say N.
+
config MMC_TIFM_SD
tristate "TI Flash Media MMC/SD Interface support (EXPERIMENTAL)"
depends on EXPERIMENTAL && PCI
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index e834fb2..30aa686 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_MMC_ARMMMCI) += mmci.o
obj-$(CONFIG_MMC_PXA) += pxamci.o
obj-$(CONFIG_MMC_IMX) += imxmmc.o
obj-$(CONFIG_MMC_MXC) += mxcmmc.o
+obj-$(CONFIG_MMC_MXS) += mxs-mmc.o
obj-$(CONFIG_MMC_SDHCI) += sdhci.o
obj-$(CONFIG_MMC_SDHCI_PCI) += sdhci-pci.o
obj-$(CONFIG_MMC_SDHCI_PXA) += sdhci-pxa.o
diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
new file mode 100644
index 0000000..4bcc5f0
--- /dev/null
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -0,0 +1,884 @@
+/*
+ * Portions copyright (C) 2003 Russell King, PXA MMCI Driver
+ * Portions copyright (C) 2004-2005 Pierre Ossman, W83L51xD SD/MMC driver
+ *
+ * Copyright 2008 Embedded Alley Solutions, Inc.
+ * Copyright 2009-2011 Freescale Semiconductor, Inc.
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/ioport.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/highmem.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/completion.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/mmc.h>
+#include <linux/mmc/sdio.h>
+#include <linux/gpio.h>
+#include <linux/regulator/consumer.h>
+
+#include <mach/mxs.h>
+#include <mach/common.h>
+#include <mach/dma.h>
+#include <mach/mmc.h>
+
+#define DRIVER_NAME "mxs-mmc"
+
+/* card detect polling timeout */
+#define MXS_MMC_DETECT_TIMEOUT (HZ/2)
+
+#define SSP_VERSION_LATEST 4
+#define ssp_is_old() (host->version < SSP_VERSION_LATEST)
+
+/* SSP registers */
+#define HW_SSP_CTRL0 0x000
+#define BM_SSP_CTRL0_RUN (1 << 29)
+#define BM_SSP_CTRL0_SDIO_IRQ_CHECK (1 << 28)
+#define BM_SSP_CTRL0_IGNORE_CRC (1 << 26)
+#define BM_SSP_CTRL0_READ (1 << 25)
+#define BM_SSP_CTRL0_DATA_XFER (1 << 24)
+#define BP_SSP_CTRL0_BUS_WIDTH (22)
+#define BM_SSP_CTRL0_BUS_WIDTH (0x3 << 22)
+#define BM_SSP_CTRL0_WAIT_FOR_IRQ (1 << 21)
+#define BM_SSP_CTRL0_LONG_RESP (1 << 19)
+#define BM_SSP_CTRL0_GET_RESP (1 << 17)
+#define BM_SSP_CTRL0_ENABLE (1 << 16)
+#define BP_SSP_CTRL0_XFER_COUNT (0)
+#define BM_SSP_CTRL0_XFER_COUNT (0xffff)
+#define HW_SSP_CMD0 0x010
+#define BM_SSP_CMD0_DBL_DATA_RATE_EN (1 << 25)
+#define BM_SSP_CMD0_SLOW_CLKING_EN (1 << 22)
+#define BM_SSP_CMD0_CONT_CLKING_EN (1 << 21)
+#define BM_SSP_CMD0_APPEND_8CYC (1 << 20)
+#define BP_SSP_CMD0_BLOCK_SIZE (16)
+#define BM_SSP_CMD0_BLOCK_SIZE (0xf << 16)
+#define BP_SSP_CMD0_BLOCK_COUNT (8)
+#define BM_SSP_CMD0_BLOCK_COUNT (0xff << 8)
+#define BP_SSP_CMD0_CMD (0)
+#define BM_SSP_CMD0_CMD (0xff)
+#define HW_SSP_CMD1 0x020
+#define HW_SSP_XFER_SIZE 0x030
+#define HW_SSP_BLOCK_SIZE 0x040
+#define BP_SSP_BLOCK_SIZE_BLOCK_COUNT (4)
+#define BM_SSP_BLOCK_SIZE_BLOCK_COUNT (0xffffff << 4)
+#define BP_SSP_BLOCK_SIZE_BLOCK_SIZE (0)
+#define BM_SSP_BLOCK_SIZE_BLOCK_SIZE (0xf)
+#define HW_SSP_TIMING (ssp_is_old() ? 0x050 : 0x070)
+#define BP_SSP_TIMING_TIMEOUT (16)
+#define BM_SSP_TIMING_TIMEOUT (0xffff << 16)
+#define BP_SSP_TIMING_CLOCK_DIVIDE (8)
+#define BM_SSP_TIMING_CLOCK_DIVIDE (0xff << 8)
+#define BP_SSP_TIMING_CLOCK_RATE (0)
+#define BM_SSP_TIMING_CLOCK_RATE (0xff)
+#define HW_SSP_CTRL1 (ssp_is_old() ? 0x060 : 0x080)
+#define BM_SSP_CTRL1_SDIO_IRQ (1 << 31)
+#define BM_SSP_CTRL1_SDIO_IRQ_EN (1 << 30)
+#define BM_SSP_CTRL1_RESP_ERR_IRQ (1 << 29)
+#define BM_SSP_CTRL1_RESP_ERR_IRQ_EN (1 << 28)
+#define BM_SSP_CTRL1_RESP_TIMEOUT_IRQ (1 << 27)
+#define BM_SSP_CTRL1_RESP_TIMEOUT_IRQ_EN (1 << 26)
+#define BM_SSP_CTRL1_DATA_TIMEOUT_IRQ (1 << 25)
+#define BM_SSP_CTRL1_DATA_TIMEOUT_IRQ_EN (1 << 24)
+#define BM_SSP_CTRL1_DATA_CRC_IRQ (1 << 23)
+#define BM_SSP_CTRL1_DATA_CRC_IRQ_EN (1 << 22)
+#define BM_SSP_CTRL1_FIFO_UNDERRUN_IRQ (1 << 21)
+#define BM_SSP_CTRL1_FIFO_UNDERRUN_IRQ_EN (1 << 20)
+#define BM_SSP_CTRL1_RECV_TIMEOUT_IRQ (1 << 17)
+#define BM_SSP_CTRL1_RECV_TIMEOUT_IRQ_EN (1 << 16)
+#define BM_SSP_CTRL1_FIFO_OVERRUN_IRQ (1 << 15)
+#define BM_SSP_CTRL1_FIFO_OVERRUN_IRQ_EN (1 << 14)
+#define BM_SSP_CTRL1_DMA_ENABLE (1 << 13)
+#define BM_SSP_CTRL1_POLARITY (1 << 9)
+#define BP_SSP_CTRL1_WORD_LENGTH (4)
+#define BM_SSP_CTRL1_WORD_LENGTH (0xf << 4)
+#define BP_SSP_CTRL1_SSP_MODE (0)
+#define BM_SSP_CTRL1_SSP_MODE (0xf)
+#define HW_SSP_SDRESP0 (ssp_is_old() ? 0x080 : 0x0a0)
+#define HW_SSP_SDRESP1 (ssp_is_old() ? 0x090 : 0x0b0)
+#define HW_SSP_SDRESP2 (ssp_is_old() ? 0x0a0 : 0x0c0)
+#define HW_SSP_SDRESP3 (ssp_is_old() ? 0x0b0 : 0x0d0)
+#define HW_SSP_STATUS (ssp_is_old() ? 0x0c0 : 0x100)
+#define BM_SSP_STATUS_CARD_DETECT (1 << 28)
+#define BM_SSP_STATUS_SDIO_IRQ (1 << 17)
+#define BM_SSP_STATUS_RESP_CRC_ERR (1 << 16)
+#define BM_SSP_STATUS_RESP_ERR (1 << 15)
+#define BM_SSP_STATUS_RESP_TIMEOUT (1 << 14)
+#define BM_SSP_STATUS_TIMEOUT (1 << 12)
+#define HW_SSP_VERSION (cpu_is_mx23() ? 0x110 : 0x130)
+#define BP_SSP_VERSION_MAJOR (24)
+
+#define BF_SSP(value, field) (((value) << BP_##field) & BM_##field)
+
+#define MXS_MMC_IRQ_BITS (BM_SSP_CTRL1_SDIO_IRQ | \
+ BM_SSP_CTRL1_RESP_ERR_IRQ | \
+ BM_SSP_CTRL1_RESP_TIMEOUT_IRQ | \
+ BM_SSP_CTRL1_DATA_TIMEOUT_IRQ | \
+ BM_SSP_CTRL1_DATA_CRC_IRQ | \
+ BM_SSP_CTRL1_FIFO_UNDERRUN_IRQ | \
+ BM_SSP_CTRL1_RECV_TIMEOUT_IRQ | \
+ BM_SSP_CTRL1_FIFO_OVERRUN_IRQ)
+
+#define MXS_MMC_ERR_BITS (BM_SSP_CTRL1_RESP_ERR_IRQ | \
+ BM_SSP_CTRL1_RESP_TIMEOUT_IRQ | \
+ BM_SSP_CTRL1_DATA_TIMEOUT_IRQ | \
+ BM_SSP_CTRL1_DATA_CRC_IRQ | \
+ BM_SSP_CTRL1_RECV_TIMEOUT_IRQ)
+
+struct mxs_mmc_host {
+ struct device *dev;
+ struct mmc_host *mmc;
+ struct resource *res;
+ struct resource *dma_res;
+ struct clk *clk;
+ unsigned int clk_rate;
+ unsigned int version;
+
+ struct mmc_request *mrq;
+ struct mmc_command *cmd;
+ struct mmc_data *data;
+
+ unsigned char bus_width;
+ unsigned present:1;
+ struct timer_list timer;
+
+ void __iomem *base;
+ int irq;
+ struct dma_chan *dmach;
+ struct dma_async_tx_descriptor *desc;
+ unsigned int dma_dir;
+
+ struct mxs_dma_data dma_data;
+ struct completion done;
+ u32 status;
+ spinlock_t lock;
+ int sdio_irq_en;
+};
+
+#define SSP_PIO_NUM 3
+static u32 ssp_pio_words[SSP_PIO_NUM];
+
+static int mxs_mmc_get_ro(struct mmc_host *mmc)
+{
+ struct mxs_mmc_host *host = mmc_priv(mmc);
+ struct mxs_mmc_platform_data *pdata = host->dev->platform_data;
+
+ if (!pdata)
+ return -EFAULT;
+
+ if (!gpio_is_valid(pdata->wp_gpio))
+ return -EINVAL;
+
+ return gpio_get_value(pdata->wp_gpio);
+}
+
+static void mxs_mmc_reset(struct mxs_mmc_host *host)
+{
+ u32 ctrl0, ctrl1;
+
+ mxs_reset_block(host->base);
+
+ ctrl0 = BM_SSP_CTRL0_IGNORE_CRC;
+ ctrl1 = BF_SSP(0x3, SSP_CTRL1_SSP_MODE) |
+ BF_SSP(0x7, SSP_CTRL1_WORD_LENGTH) |
+ BM_SSP_CTRL1_DMA_ENABLE |
+ BM_SSP_CTRL1_POLARITY |
+ BM_SSP_CTRL1_RECV_TIMEOUT_IRQ_EN |
+ BM_SSP_CTRL1_DATA_CRC_IRQ_EN |
+ BM_SSP_CTRL1_DATA_TIMEOUT_IRQ_EN |
+ BM_SSP_CTRL1_RESP_TIMEOUT_IRQ_EN |
+ BM_SSP_CTRL1_RESP_ERR_IRQ_EN;
+
+ __raw_writel(BF_SSP(0xffff, SSP_TIMING_TIMEOUT) |
+ BF_SSP(2, SSP_TIMING_CLOCK_DIVIDE) |
+ BF_SSP(0, SSP_TIMING_CLOCK_RATE),
+ host->base + HW_SSP_TIMING);
+
+ if (host->sdio_irq_en) {
+ ctrl0 |= BM_SSP_CTRL0_SDIO_IRQ_CHECK;
+ ctrl1 |= BM_SSP_CTRL1_SDIO_IRQ_EN;
+ }
+
+ __raw_writel(ctrl0, host->base + HW_SSP_CTRL0);
+ __raw_writel(ctrl1, host->base + HW_SSP_CTRL1);
+}
+
+static inline int mxs_mmc_is_plugged(struct mxs_mmc_host *host)
+{
+ return !(__raw_readl(host->base + HW_SSP_STATUS) &
+ BM_SSP_STATUS_CARD_DETECT);
+}
+
+static void mxs_mmc_detect_poll(unsigned long arg)
+{
+ struct mxs_mmc_host *host = (struct mxs_mmc_host *)arg;
+ int card_status;
+
+ card_status = mxs_mmc_is_plugged(host);
+ if (card_status != host->present) {
+ mxs_mmc_reset(host);
+ host->present = card_status;
+ mmc_detect_change(host->mmc, 0);
+ }
+
+ mod_timer(&host->timer, jiffies + MXS_MMC_DETECT_TIMEOUT);
+}
+
+static void mxs_mmc_dma_irq_callback(void *param)
+{
+ struct mxs_mmc_host *host = param;
+
+ host->status = __raw_readl(host->base + HW_SSP_STATUS);
+ complete(&host->done);
+}
+
+static irqreturn_t mxs_mmc_irq_handler(int irq, void *dev_id)
+{
+ struct mxs_mmc_host *host = dev_id;
+ u32 stat;
+
+ stat = __raw_readl(host->base + HW_SSP_CTRL1);
+ __mxs_clrl(stat & MXS_MMC_IRQ_BITS, host->base + HW_SSP_CTRL1);
+
+ if (host->cmd && (stat & MXS_MMC_ERR_BITS))
+ host->status = __raw_readl(host->base + HW_SSP_STATUS);
+
+ if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN))
+ mmc_signal_sdio_irq(host->mmc);
+
+ return IRQ_HANDLED;
+}
+
+static inline int mxs_mmc_cmd_error(u32 status)
+{
+ int err = 0;
+
+ if (status & BM_SSP_STATUS_TIMEOUT)
+ err = -ETIMEDOUT;
+ else if (status & BM_SSP_STATUS_RESP_TIMEOUT)
+ err = -ETIMEDOUT;
+ else if (status & BM_SSP_STATUS_RESP_CRC_ERR)
+ err = -EILSEQ;
+ else if (status & BM_SSP_STATUS_RESP_ERR)
+ err = -EIO;
+
+ return err;
+}
+
+static void mxs_mmc_do_dma(struct mxs_mmc_host *host,
+ enum dma_data_direction direction, unsigned int wait4end)
+{
+ struct mmc_command *cmd = host->cmd;
+ struct mmc_data *data = cmd->data;
+ struct scatterlist * sgl;
+ unsigned int sg_len;
+
+ if (direction == DMA_NONE) {
+ /* pio dma */
+ sgl = (struct scatterlist *) ssp_pio_words;
+ sg_len = SSP_PIO_NUM;
+ } else {
+ /* data dma */
+ dma_map_sg(host->dev, data->sg, data->sg_len, direction);
+ sgl = data->sg;
+ sg_len = data->sg_len;
+ }
+
+ host->desc = host->dmach->device->device_prep_slave_sg(host->dmach,
+ sgl, sg_len, direction, wait4end);
+ if (!host->desc) {
+ dev_err(host->dev, "%s: failed to setup dma\n", __func__);
+ goto out_dma_unmap;
+ }
+
+ host->desc->callback = mxs_mmc_dma_irq_callback;
+ host->desc->callback_param = host;
+
+ init_completion(&host->done);
+ dmaengine_submit(host->desc);
+ wait_for_completion(&host->done);
+
+out_dma_unmap:
+ if (direction != DMA_NONE)
+ dma_unmap_sg(host->dev, data->sg, data->sg_len, direction);
+}
+
+static void mxs_mmc_bc(struct mxs_mmc_host *host)
+{
+ struct mmc_command *cmd = host->cmd;
+ u32 ctrl0, cmd0, cmd1;
+
+ ctrl0 = BM_SSP_CTRL0_ENABLE | BM_SSP_CTRL0_IGNORE_CRC;
+ cmd0 = BF_SSP(cmd->opcode, SSP_CMD0_CMD) | BM_SSP_CMD0_APPEND_8CYC;
+ cmd1 = cmd->arg;
+
+ if (host->sdio_irq_en) {
+ ctrl0 |= BM_SSP_CTRL0_SDIO_IRQ_CHECK;
+ cmd0 |= BM_SSP_CMD0_CONT_CLKING_EN | BM_SSP_CMD0_SLOW_CLKING_EN;
+ }
+
+ ssp_pio_words[0] = ctrl0;
+ ssp_pio_words[1] = cmd0;
+ ssp_pio_words[2] = cmd1;
+ mxs_mmc_do_dma(host, DMA_NONE, 1);
+
+ cmd->error = mxs_mmc_cmd_error(host->status);
+ if (cmd->error)
+ dev_dbg(host->dev, "%s: command error %d, status 0x%08x\n",
+ __func__, cmd->error, host->status);
+}
+
+static void mxs_mmc_ac(struct mxs_mmc_host *host)
+{
+ struct mmc_command *cmd = host->cmd;
+ u32 ignore_crc, get_resp, long_resp;
+ u32 ctrl0, cmd0, cmd1;
+
+ ignore_crc = (mmc_resp_type(cmd) & MMC_RSP_CRC) ?
+ 0 : BM_SSP_CTRL0_IGNORE_CRC;
+ get_resp = (mmc_resp_type(cmd) & MMC_RSP_PRESENT) ?
+ BM_SSP_CTRL0_GET_RESP : 0;
+ long_resp = (mmc_resp_type(cmd) & MMC_RSP_136) ?
+ BM_SSP_CTRL0_LONG_RESP : 0;
+
+ ctrl0 = BM_SSP_CTRL0_ENABLE | ignore_crc | get_resp | long_resp;
+ cmd0 = BF_SSP(cmd->opcode, SSP_CMD0_CMD);
+ cmd1 = cmd->arg;
+
+ if (host->sdio_irq_en) {
+ ctrl0 |= BM_SSP_CTRL0_SDIO_IRQ_CHECK;
+ cmd0 |= BM_SSP_CMD0_CONT_CLKING_EN | BM_SSP_CMD0_SLOW_CLKING_EN;
+ }
+
+ ssp_pio_words[0] = ctrl0;
+ ssp_pio_words[1] = cmd0;
+ ssp_pio_words[2] = cmd1;
+ mxs_mmc_do_dma(host, DMA_NONE, 1);
+
+ switch (mmc_resp_type(cmd)) {
+ case MMC_RSP_NONE:
+ while (__raw_readl(host->base + HW_SSP_CTRL0)
+ & BM_SSP_CTRL0_RUN)
+ continue;
+ break;
+ case MMC_RSP_R1:
+ case MMC_RSP_R1B:
+ case MMC_RSP_R3:
+ cmd->resp[0] = __raw_readl(host->base + HW_SSP_SDRESP0);
+ break;
+ case MMC_RSP_R2:
+ cmd->resp[3] = __raw_readl(host->base + HW_SSP_SDRESP0);
+ cmd->resp[2] = __raw_readl(host->base + HW_SSP_SDRESP1);
+ cmd->resp[1] = __raw_readl(host->base + HW_SSP_SDRESP2);
+ cmd->resp[0] = __raw_readl(host->base + HW_SSP_SDRESP3);
+ break;
+ default:
+ dev_warn(host->dev, "unsupported response type 0x%x\n",
+ mmc_resp_type(cmd));
+ BUG();
+ break;
+ }
+
+ cmd->error = mxs_mmc_cmd_error(host->status);
+ if (cmd->error)
+ dev_dbg(host->dev, "%s: command error %d, status 0x%08x\n",
+ __func__, cmd->error, host->status);
+}
+
+static unsigned short mxs_ns_to_ssp_ticks(unsigned clock_rate, unsigned ns)
+{
+ const unsigned int ssp_timeout_mul = 4096;
+ /*
+ * Calculate ticks in ms since ns are large numbers
+ * and might overflow
+ */
+ const unsigned int clock_per_ms = clock_rate / 1000;
+ const unsigned int ms = ns / 1000;
+ const unsigned int ticks = ms * clock_per_ms;
+ const unsigned int ssp_ticks = ticks / ssp_timeout_mul;
+
+ BUG_ON(ssp_ticks == 0);
+ return ssp_ticks;
+}
+
+static void mxs_mmc_adtc(struct mxs_mmc_host *host)
+{
+ struct mmc_command *cmd = host->cmd;
+ u32 ignore_crc, get_resp, long_resp, read;
+ u32 ctrl0, cmd0, cmd1;
+ u32 timeout, val;
+ u32 data_size = cmd->data->blksz * cmd->data->blocks;
+ u32 log2_blksz;
+
+ ignore_crc = (mmc_resp_type(cmd) & MMC_RSP_CRC) ?
+ 0 : BM_SSP_CTRL0_IGNORE_CRC;
+ get_resp = (mmc_resp_type(cmd) & MMC_RSP_PRESENT) ?
+ BM_SSP_CTRL0_GET_RESP : 0;
+ long_resp = (mmc_resp_type(cmd) & MMC_RSP_136) ?
+ BM_SSP_CTRL0_LONG_RESP : 0;
+
+ if (cmd->data->flags & MMC_DATA_WRITE) {
+ host->dma_dir = DMA_TO_DEVICE;
+ read = 0;
+ } else {
+ host->dma_dir = DMA_FROM_DEVICE;
+ read = BM_SSP_CTRL0_READ;
+ }
+
+ ctrl0 = BF_SSP(host->bus_width, SSP_CTRL0_BUS_WIDTH) |
+ ignore_crc | get_resp | long_resp |
+ BM_SSP_CTRL0_DATA_XFER | read |
+ BM_SSP_CTRL0_WAIT_FOR_IRQ |
+ BM_SSP_CTRL0_ENABLE;
+
+ cmd0 = BF_SSP(cmd->opcode, SSP_CMD0_CMD);
+
+ /* get logarithm to base 2 of block size for setting register */
+ log2_blksz = ilog2(cmd->data->blksz);
+
+ /* xfer count, block size and count need to be set differently */
+ if (ssp_is_old()) {
+ ctrl0 |= BF_SSP(data_size, SSP_CTRL0_XFER_COUNT);
+ cmd0 |= BF_SSP(log2_blksz, SSP_CMD0_BLOCK_SIZE) |
+ BF_SSP(cmd->data->blocks - 1, SSP_CMD0_BLOCK_COUNT);
+ } else {
+ __raw_writel(data_size, host->base + HW_SSP_XFER_SIZE);
+ __raw_writel(BF_SSP(log2_blksz, SSP_BLOCK_SIZE_BLOCK_SIZE) |
+ BF_SSP(cmd->data->blocks - 1, SSP_BLOCK_SIZE_BLOCK_COUNT),
+ host->base + HW_SSP_BLOCK_SIZE);
+ }
+
+ if ((cmd->opcode == MMC_STOP_TRANSMISSION) ||
+ (cmd->opcode == SD_IO_RW_EXTENDED))
+ cmd0 |= BM_SSP_CMD0_APPEND_8CYC;
+
+ cmd1 = cmd->arg;
+
+ if (host->sdio_irq_en) {
+ ctrl0 |= BM_SSP_CTRL0_SDIO_IRQ_CHECK;
+ cmd0 |= BM_SSP_CMD0_CONT_CLKING_EN | BM_SSP_CMD0_SLOW_CLKING_EN;
+ }
+
+ /* set the timeout count */
+ timeout = mxs_ns_to_ssp_ticks(host->clk_rate, cmd->data->timeout_ns);
+ val = __raw_readl(host->base + HW_SSP_TIMING);
+ val &= ~(BM_SSP_TIMING_TIMEOUT);
+ val |= BF_SSP(timeout, SSP_TIMING_TIMEOUT);
+ __raw_writel(val, host->base + HW_SSP_TIMING);
+
+ /* pio dma */
+ ssp_pio_words[0] = ctrl0;
+ ssp_pio_words[1] = cmd0;
+ ssp_pio_words[2] = cmd1;
+ mxs_mmc_do_dma(host, DMA_NONE, 0);
+
+ switch (mmc_resp_type(cmd)) {
+ case MMC_RSP_NONE:
+ break;
+ case MMC_RSP_R1:
+ case MMC_RSP_R3:
+ cmd->resp[0] = __raw_readl(host->base + HW_SSP_SDRESP0);
+ break;
+ case MMC_RSP_R2:
+ cmd->resp[3] = __raw_readl(host->base + HW_SSP_SDRESP0);
+ cmd->resp[2] = __raw_readl(host->base + HW_SSP_SDRESP1);
+ cmd->resp[1] = __raw_readl(host->base + HW_SSP_SDRESP2);
+ cmd->resp[0] = __raw_readl(host->base + HW_SSP_SDRESP3);
+ break;
+ default:
+ dev_warn(host->dev, "unsupported response type 0x%x\n",
+ mmc_resp_type(cmd));
+ BUG();
+ break;
+ }
+
+ /* data dma */
+ mxs_mmc_do_dma(host, host->dma_dir, 1);
+
+ cmd->data->bytes_xfered = data_size;
+
+ cmd->error = mxs_mmc_cmd_error(host->status);
+ if (cmd->error)
+ dev_dbg(host->dev, "%s: command error %d, status 0x%08x\n",
+ __func__, cmd->error, host->status);
+}
+
+static void mxs_mmc_start_cmd(struct mxs_mmc_host *host,
+ struct mmc_command *cmd)
+{
+ host->cmd = cmd;
+
+ switch (mmc_cmd_type(cmd)) {
+ case MMC_CMD_BC:
+ mxs_mmc_bc(host);
+ break;
+ case MMC_CMD_BCR:
+ mxs_mmc_ac(host);
+ break;
+ case MMC_CMD_AC:
+ mxs_mmc_ac(host);
+ break;
+ case MMC_CMD_ADTC:
+ mxs_mmc_adtc(host);
+ break;
+ default:
+ dev_warn(host->dev, "unknown MMC command\n");
+ BUG();
+ break;
+ }
+}
+
+static void mxs_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
+{
+ struct mxs_mmc_host *host = mmc_priv(mmc);
+
+ BUG_ON(host->mrq != NULL);
+
+ host->mrq = mrq;
+ mxs_mmc_start_cmd(host, mrq->cmd);
+
+ if (mrq->data && mrq->data->stop) {
+ dev_dbg(host->dev, "Stop opcode is %u\n",
+ mrq->data->stop->opcode);
+ mxs_mmc_start_cmd(host, mrq->data->stop);
+ }
+
+ host->mrq = NULL;
+ mmc_request_done(mmc, mrq);
+}
+
+static void mxs_mmc_set_clk_rate(struct mxs_mmc_host *host, unsigned int rate)
+{
+ unsigned int ssp_rate, bit_rate;
+ u32 div1, div2;
+ u32 val;
+
+ ssp_rate = clk_get_rate(host->clk);
+
+ for (div1 = 2; div1 < 254; div1 += 2) {
+ div2 = ssp_rate / rate / div1;
+ if (div2 < 0x100)
+ break;
+ }
+
+ if (div1 >= 254) {
+ dev_err(host->dev, "cannot set clock to %d\n", rate);
+ return;
+ }
+
+ if (div2 == 0)
+ bit_rate = ssp_rate / div1;
+ else
+ bit_rate = ssp_rate / div1 / div2;
+
+ val = __raw_readl(host->base + HW_SSP_TIMING);
+ val &= ~(BM_SSP_TIMING_CLOCK_DIVIDE | BM_SSP_TIMING_CLOCK_RATE);
+ val |= BF_SSP(div1, SSP_TIMING_CLOCK_DIVIDE);
+ val |= BF_SSP(div2 - 1, SSP_TIMING_CLOCK_RATE);
+ __raw_writel(val, host->base + HW_SSP_TIMING);
+
+ host->clk_rate = bit_rate;
+
+ dev_dbg(host->dev, "div1 %d, div2 %d, ssp %d, bit %d, rate %d\n",
+ div1, div2, ssp_rate, bit_rate, rate);
+}
+
+static void mxs_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+ struct mxs_mmc_host *host = mmc_priv(mmc);
+
+ if (ios->bus_width == MMC_BUS_WIDTH_8)
+ host->bus_width = 2;
+ else if (ios->bus_width == MMC_BUS_WIDTH_4)
+ host->bus_width = 1;
+ else
+ host->bus_width = 0;
+
+ if (ios->clock)
+ mxs_mmc_set_clk_rate(host, ios->clock);
+}
+
+static void mxs_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
+{
+ struct mxs_mmc_host *host = mmc_priv(mmc);
+ unsigned long flags;
+
+ spin_lock_irqsave(&host->lock, flags);
+
+ host->sdio_irq_en = enable;
+
+ if (enable) {
+ __mxs_setl(BM_SSP_CTRL0_SDIO_IRQ_CHECK,
+ host->base + HW_SSP_CTRL0);
+ __mxs_setl(BM_SSP_CTRL1_SDIO_IRQ_EN,
+ host->base + HW_SSP_CTRL1);
+
+ if (__raw_readl(host->base + HW_SSP_STATUS) &
+ BM_SSP_STATUS_SDIO_IRQ)
+ mmc_signal_sdio_irq(host->mmc);
+
+ } else {
+ __mxs_clrl(BM_SSP_CTRL0_SDIO_IRQ_CHECK,
+ host->base + HW_SSP_CTRL0);
+ __mxs_clrl(BM_SSP_CTRL1_SDIO_IRQ_EN,
+ host->base + HW_SSP_CTRL1);
+ }
+
+ spin_unlock_irqrestore(&host->lock, flags);
+}
+
+static const struct mmc_host_ops mxs_mmc_ops = {
+ .request = mxs_mmc_request,
+ .get_ro = mxs_mmc_get_ro,
+ .set_ios = mxs_mmc_set_ios,
+ .enable_sdio_irq = mxs_mmc_enable_sdio_irq,
+};
+
+static bool mxs_mmc_dma_filter(struct dma_chan *chan, void *param)
+{
+ struct mxs_mmc_host *host = param;
+
+ /* apbh dma gets dev_id 0 */
+ if (chan->device->dev_id)
+ return false;
+
+ if (chan->chan_id != host->dma_res->start)
+ return false;
+
+ chan->private = &host->dma_data;
+
+ return true;
+}
+
+static int mxs_mmc_probe(struct platform_device *pdev)
+{
+ struct mxs_mmc_host *host;
+ struct mmc_host *mmc;
+ struct resource *iores, *dmares, *r;
+ int ret = 0, irq_err, irq_dma;
+ dma_cap_mask_t mask;
+
+ iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0);
+ irq_err = platform_get_irq(pdev, 0);
+ irq_dma = platform_get_irq(pdev, 1);
+ if (!iores || !dmares || irq_err < 0 || irq_dma < 0)
+ return -EINVAL;
+
+ r = request_mem_region(iores->start, resource_size(iores), pdev->name);
+ if (!r)
+ return -EBUSY;
+
+ mmc = mmc_alloc_host(sizeof(struct mxs_mmc_host), &pdev->dev);
+ if (!mmc) {
+ ret = -ENOMEM;
+ goto out_release_mem;
+ }
+
+ host = mmc_priv(mmc);
+ host->base = ioremap(r->start, resource_size(r));
+ if (!host->base) {
+ ret = -ENOMEM;
+ goto out_mmc_free;
+ }
+
+ /* only major verion does matter */
+ host->version = __raw_readl(host->base + HW_SSP_VERSION) >>
+ BP_SSP_VERSION_MAJOR;
+
+ host->mmc = mmc;
+ host->dev = &pdev->dev;
+ host->res = r;
+ host->dma_res = dmares;
+ host->irq = irq_err;
+ host->sdio_irq_en = 0;
+
+ host->clk = clk_get(&pdev->dev, NULL);
+ if (IS_ERR(host->clk)) {
+ ret = PTR_ERR(host->clk);
+ goto out_iounmap;
+ }
+ clk_enable(host->clk);
+
+ mxs_mmc_reset(host);
+
+ dma_cap_zero(mask);
+ dma_cap_set(DMA_SLAVE, mask);
+ host->dma_data.chan_irq = irq_dma;
+ host->dmach = dma_request_channel(mask, mxs_mmc_dma_filter, host);
+ if (!host->dmach) {
+ dev_err(host->dev, "failed to request dma\n");
+ goto out_clk_put;
+ }
+
+ /* set mmc core parameters */
+ mmc->ops = &mxs_mmc_ops;
+ mmc->caps = MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA |
+ MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED |
+ MMC_CAP_SDIO_IRQ;
+
+ mmc->f_min = 400000;
+ mmc->f_max = 288000000;
+ mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+
+ mmc->max_segs = 53;
+ mmc->max_blk_size = 1 << 0xf;
+ mmc->max_blk_count = (ssp_is_old()) ? 0xff : 0xffffff;
+ mmc->max_req_size = (ssp_is_old()) ? 0xffff : 0xffffffff;
+ mmc->max_seg_size = dma_get_max_seg_size(host->dmach->device->dev);
+
+ ret = request_irq(host->irq, mxs_mmc_irq_handler, 0,
+ DRIVER_NAME, host);
+ if (ret)
+ goto out_free_dma;
+
+ /* current card status */
+ host->present = mxs_mmc_is_plugged(host);
+
+ /* card detection polling timer */
+ init_timer(&host->timer);
+ host->timer.function = mxs_mmc_detect_poll;
+ host->timer.data = (unsigned long) host;
+ host->timer.expires = jiffies + MXS_MMC_DETECT_TIMEOUT;
+ add_timer(&host->timer);
+
+ platform_set_drvdata(pdev, mmc);
+
+ spin_lock_init(&host->lock);
+
+ ret = mmc_add_host(mmc);
+ if (ret)
+ goto out_del_timer;
+
+ dev_info(host->dev, "initialized\n");
+
+ return 0;
+
+out_del_timer:
+ del_timer(&host->timer);
+ free_irq(host->irq, host);
+out_free_dma:
+ if (host->dmach)
+ dma_release_channel(host->dmach);
+out_clk_put:
+ clk_disable(host->clk);
+ clk_put(host->clk);
+out_iounmap:
+ iounmap(host->base);
+out_mmc_free:
+ mmc_free_host(mmc);
+out_release_mem:
+ release_mem_region(iores->start, resource_size(iores));
+ return ret;
+}
+
+static int mxs_mmc_remove(struct platform_device *pdev)
+{
+ struct mmc_host *mmc = platform_get_drvdata(pdev);
+ struct mxs_mmc_host *host = mmc_priv(mmc);
+
+ platform_set_drvdata(pdev, NULL);
+
+ mmc_remove_host(mmc);
+
+ del_timer(&host->timer);
+
+ free_irq(host->irq, host);
+
+ if (host->dmach)
+ dma_release_channel(host->dmach);
+
+ clk_disable(host->clk);
+ clk_put(host->clk);
+
+ iounmap(host->base);
+
+ mmc_free_host(mmc);
+
+ release_mem_region(host->res->start, resource_size(host->res));
+
+ return 0;
+}
+
+#ifdef CONFIG_PM
+static int mxs_mmc_suspend(struct device *dev)
+{
+ struct mmc_host *mmc = dev_get_drvdata(dev);
+ struct mxs_mmc_host *host = mmc_priv(mmc);
+ int ret = 0;
+
+ if (mmc)
+ ret = mmc_suspend_host(mmc);
+
+ clk_disable(host->clk);
+
+ return ret;
+}
+
+static int mxs_mmc_resume(struct device *dev)
+{
+ struct mmc_host *mmc = dev_get_drvdata(dev);
+ struct mxs_mmc_host *host = mmc_priv(mmc);
+ int ret = 0;
+
+ clk_enable(host->clk);
+
+ if (mmc)
+ ret = mmc_resume_host(mmc);
+
+ return ret;
+}
+
+static const struct dev_pm_ops mxs_mmc_pm_ops = {
+ .suspend = mxs_mmc_suspend,
+ .resume = mxs_mmc_resume,
+};
+#endif
+
+static struct platform_driver mxs_mmc_driver = {
+ .probe = mxs_mmc_probe,
+ .remove = mxs_mmc_remove,
+ .driver = {
+ .name = DRIVER_NAME,
+ .owner = THIS_MODULE,
+#ifdef CONFIG_PM
+ .pm = &mxs_mmc_pm_ops,
+#endif
+ },
+};
+
+static int __init mxs_mmc_init(void)
+{
+ return platform_driver_register(&mxs_mmc_driver);
+}
+
+static void __exit mxs_mmc_exit(void)
+{
+ platform_driver_unregister(&mxs_mmc_driver);
+}
+
+module_init(mxs_mmc_init);
+module_exit(mxs_mmc_exit);
+
+MODULE_DESCRIPTION("FREESCALE MXS MMC peripheral");
+MODULE_LICENSE("GPL");
--
1.7.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-05 2:18 ` [PATCH 1/7] mmc: mxs-mmc: add mmc host " Shawn Guo
@ 2011-02-04 20:26 ` Arnd Bergmann
2011-02-11 0:35 ` Shawn Guo
2011-02-08 11:41 ` Lothar Waßmann
2011-02-09 7:46 ` Lothar Waßmann
2 siblings, 1 reply; 33+ messages in thread
From: Arnd Bergmann @ 2011-02-04 20:26 UTC (permalink / raw)
To: linux-arm-kernel
On Saturday 05 February 2011 03:18:41 Shawn Guo wrote:
> This adds the mmc host driver for Freescale MXS-based SoC i.MX23/28.
> The driver calls into mxs-dma via generic dmaengine api for both pio
> and data transfer.
Hi Shawn,
The driver looks good coding-style wise, but it's my impression that
you do some thing less efficient or more complex than necessary. To
be more specific:
> +struct mxs_mmc_host {
It seems you have a number of members in this struct that only
add confusion but are not actually needed, so just get rid of
them:
> + struct device *dev;
mmc_host->parent ?
> + struct resource *res;
> + struct resource *dma_res;
> + struct clk *clk;
are visible in the parent.
> + unsigned int version;
unused
> + struct mmc_command *cmd;
mrq->cmd ?
> + struct mmc_data *data;
unused
> + unsigned present:1;
Your card detection by polling through this variable is
really bad for power management. Is there really no interrupt
that gets triggered when installing or removing a card?
Also, please try to avoid bit fields. 'bool' variables are
fine for flags.
> + unsigned int dma_dir;
not needed, a local variable would be just fine.
> + struct mxs_dma_data dma_data;
Why do you need host specific DMA structures? Please stick to
the generic dma-engine API if possible.
> + struct completion done;
The mmc layer already comes with a generic completion that you
can and should use, mainly for performance reasons. Adding your
own completion forces every single request to go through an
additional context switch, compared to just returning from
the request function when you have scheduled the command, and
calling mmc_request_done from the interrupt handler.
> + u32 status;
Won't be needed any more when you complete the requests at
interrupt time.
> +#define SSP_PIO_NUM 3
> +static u32 ssp_pio_words[SSP_PIO_NUM];
I don't think this is safe: This is a global variable, so if you have
multiple controllers, they would access the same data. Why not integrate
it into the host data structure?
> +
> +static void mxs_mmc_reset(struct mxs_mmc_host *host)
> +{
> + u32 ctrl0, ctrl1;
> +
> + mxs_reset_block(host->base);
> +
> + ctrl0 = BM_SSP_CTRL0_IGNORE_CRC;
> + ctrl1 = BF_SSP(0x3, SSP_CTRL1_SSP_MODE) |
> + BF_SSP(0x7, SSP_CTRL1_WORD_LENGTH) |
> + BM_SSP_CTRL1_DMA_ENABLE |
> + BM_SSP_CTRL1_POLARITY |
> + BM_SSP_CTRL1_RECV_TIMEOUT_IRQ_EN |
> + BM_SSP_CTRL1_DATA_CRC_IRQ_EN |
> + BM_SSP_CTRL1_DATA_TIMEOUT_IRQ_EN |
> + BM_SSP_CTRL1_RESP_TIMEOUT_IRQ_EN |
> + BM_SSP_CTRL1_RESP_ERR_IRQ_EN;
> +
> + __raw_writel(BF_SSP(0xffff, SSP_TIMING_TIMEOUT) |
> + BF_SSP(2, SSP_TIMING_CLOCK_DIVIDE) |
> + BF_SSP(0, SSP_TIMING_CLOCK_RATE),
> + host->base + HW_SSP_TIMING);
Please use proper MMIO accessors like writel() and readl() that
are known to be safe here. The __raw_ variants are not really
meant for device drivers, which can lead to very subtle bugs.
> +static irqreturn_t mxs_mmc_irq_handler(int irq, void *dev_id)
> +{
> + struct mxs_mmc_host *host = dev_id;
> + u32 stat;
> +
> + stat = __raw_readl(host->base + HW_SSP_CTRL1);
> + __mxs_clrl(stat & MXS_MMC_IRQ_BITS, host->base + HW_SSP_CTRL1);
> +
> + if (host->cmd && (stat & MXS_MMC_ERR_BITS))
> + host->status = __raw_readl(host->base + HW_SSP_STATUS);
> +
> + if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN))
> + mmc_signal_sdio_irq(host->mmc);
> +
> + return IRQ_HANDLED;
> +}
You use spin_lock_irqsave in mxs_mmc_enable_sdio_irq, but don't
actually use the spinlock in the interrupt handler. This means
that either your interrupt handler is broken because it doesn't
lock, or that you don't actually need the _irqsave part.
> +static void mxs_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
> +{
> + struct mxs_mmc_host *host = mmc_priv(mmc);
> +
> + BUG_ON(host->mrq != NULL);
> +
> + host->mrq = mrq;
> + mxs_mmc_start_cmd(host, mrq->cmd);
> +
> + if (mrq->data && mrq->data->stop) {
> + dev_dbg(host->dev, "Stop opcode is %u\n",
> + mrq->data->stop->opcode);
> + mxs_mmc_start_cmd(host, mrq->data->stop);
> + }
> +
> + host->mrq = NULL;
> + mmc_request_done(mmc, mrq);
> +}
As mentioned above, the mmc_request_done() shouldn't really be needed here,
it's more efficient to do it in the interrupt handler.
Arnd
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-04 20:26 ` Arnd Bergmann
@ 2011-02-11 0:35 ` Shawn Guo
2011-02-10 17:17 ` Arnd Bergmann
2011-02-13 17:53 ` Shawn Guo
0 siblings, 2 replies; 33+ messages in thread
From: Shawn Guo @ 2011-02-11 0:35 UTC (permalink / raw)
To: linux-arm-kernel
Hi Arnd,
Sorry for the late response. It really took me some time to address
your comments, especially about using mmc core completion to save
the one in the driver.
On Fri, Feb 04, 2011 at 09:26:52PM +0100, Arnd Bergmann wrote:
> On Saturday 05 February 2011 03:18:41 Shawn Guo wrote:
> > This adds the mmc host driver for Freescale MXS-based SoC i.MX23/28.
> > The driver calls into mxs-dma via generic dmaengine api for both pio
> > and data transfer.
>
> Hi Shawn,
>
> The driver looks good coding-style wise, but it's my impression that
> you do some thing less efficient or more complex than necessary. To
> be more specific:
Thanks for the review and comments.
>
> > +struct mxs_mmc_host {
>
> It seems you have a number of members in this struct that only
> add confusion but are not actually needed, so just get rid of
> them:
>
> > + struct device *dev;
>
> mmc_host->parent ?
>
OK. Will replace all host->dev with mmc_dev(host->mmc) to save it.
> > + struct resource *res;
> > + struct resource *dma_res;
> > + struct clk *clk;
>
> are visible in the parent.
>
It seems this is a generally used approach, seen in mxcmmc and pxamci.
> > + unsigned int version;
>
> unused
>
#define ssp_is_old() (host->version < SSP_VERSION_LATEST)
> > + struct mmc_command *cmd;
>
> mrq->cmd ?
>
Generally used approach again.
> > + struct mmc_data *data;
>
> unused
>
Will be used in v2.
> > + unsigned present:1;
>
> Your card detection by polling through this variable is
> really bad for power management. Is there really no interrupt
> that gets triggered when installing or removing a card?
>
Good point. Will try to use interrupt.
> Also, please try to avoid bit fields. 'bool' variables are
> fine for flags.
>
> > + unsigned int dma_dir;
>
> not needed, a local variable would be just fine.
>
Will be needed in v2.
> > + struct mxs_dma_data dma_data;
>
> Why do you need host specific DMA structures? Please stick to
> the generic dma-engine API if possible.
>
I'm sticking to the generic dmaengine api. The mxs dma hardware
has separate irq line for every single dma channel, which needs to
be told by client driver.
> > + struct completion done;
>
> The mmc layer already comes with a generic completion that you
> can and should use, mainly for performance reasons. Adding your
> own completion forces every single request to go through an
> additional context switch, compared to just returning from
> the request function when you have scheduled the command, and
> calling mmc_request_done from the interrupt handler.
>
I'm rewriting some amount of codes to address it.
> > + u32 status;
>
> Won't be needed any more when you complete the requests at
> interrupt time.
>
Yes.
> > +#define SSP_PIO_NUM 3
> > +static u32 ssp_pio_words[SSP_PIO_NUM];
>
> I don't think this is safe: This is a global variable, so if you have
> multiple controllers, they would access the same data. Why not integrate
> it into the host data structure?
>
Sounds sane.
> > +
> > +static void mxs_mmc_reset(struct mxs_mmc_host *host)
> > +{
> > + u32 ctrl0, ctrl1;
> > +
> > + mxs_reset_block(host->base);
> > +
> > + ctrl0 = BM_SSP_CTRL0_IGNORE_CRC;
> > + ctrl1 = BF_SSP(0x3, SSP_CTRL1_SSP_MODE) |
> > + BF_SSP(0x7, SSP_CTRL1_WORD_LENGTH) |
> > + BM_SSP_CTRL1_DMA_ENABLE |
> > + BM_SSP_CTRL1_POLARITY |
> > + BM_SSP_CTRL1_RECV_TIMEOUT_IRQ_EN |
> > + BM_SSP_CTRL1_DATA_CRC_IRQ_EN |
> > + BM_SSP_CTRL1_DATA_TIMEOUT_IRQ_EN |
> > + BM_SSP_CTRL1_RESP_TIMEOUT_IRQ_EN |
> > + BM_SSP_CTRL1_RESP_ERR_IRQ_EN;
> > +
> > + __raw_writel(BF_SSP(0xffff, SSP_TIMING_TIMEOUT) |
> > + BF_SSP(2, SSP_TIMING_CLOCK_DIVIDE) |
> > + BF_SSP(0, SSP_TIMING_CLOCK_RATE),
> > + host->base + HW_SSP_TIMING);
>
> Please use proper MMIO accessors like writel() and readl() that
> are known to be safe here. The __raw_ variants are not really
> meant for device drivers, which can lead to very subtle bugs.
>
OK. __mxs_clrl/setl will also be replaced by writel with set/clr
register offset, so that code looks more consistent.
> > +static irqreturn_t mxs_mmc_irq_handler(int irq, void *dev_id)
> > +{
> > + struct mxs_mmc_host *host = dev_id;
> > + u32 stat;
> > +
> > + stat = __raw_readl(host->base + HW_SSP_CTRL1);
> > + __mxs_clrl(stat & MXS_MMC_IRQ_BITS, host->base + HW_SSP_CTRL1);
> > +
> > + if (host->cmd && (stat & MXS_MMC_ERR_BITS))
> > + host->status = __raw_readl(host->base + HW_SSP_STATUS);
> > +
> > + if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN))
> > + mmc_signal_sdio_irq(host->mmc);
> > +
> > + return IRQ_HANDLED;
> > +}
>
> You use spin_lock_irqsave in mxs_mmc_enable_sdio_irq, but don't
> actually use the spinlock in the interrupt handler. This means
> that either your interrupt handler is broken because it doesn't
> lock, or that you don't actually need the _irqsave part.
>
I do not understand this one. I'm seeing mxcmmc and pxamci use
spin_lock_irqsave/irqrestore in the similar way.
> > +static void mxs_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
> > +{
> > + struct mxs_mmc_host *host = mmc_priv(mmc);
> > +
> > + BUG_ON(host->mrq != NULL);
> > +
> > + host->mrq = mrq;
> > + mxs_mmc_start_cmd(host, mrq->cmd);
> > +
> > + if (mrq->data && mrq->data->stop) {
> > + dev_dbg(host->dev, "Stop opcode is %u\n",
> > + mrq->data->stop->opcode);
> > + mxs_mmc_start_cmd(host, mrq->data->stop);
> > + }
> > +
> > + host->mrq = NULL;
> > + mmc_request_done(mmc, mrq);
> > +}
>
> As mentioned above, the mmc_request_done() shouldn't really be needed here,
> it's more efficient to do it in the interrupt handler.
>
OK.
Regards,
Shawn
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-11 0:35 ` Shawn Guo
@ 2011-02-10 17:17 ` Arnd Bergmann
2011-02-11 23:14 ` Shawn Guo
2011-02-13 17:53 ` Shawn Guo
1 sibling, 1 reply; 33+ messages in thread
From: Arnd Bergmann @ 2011-02-10 17:17 UTC (permalink / raw)
To: linux-arm-kernel
On Friday 11 February 2011, Shawn Guo wrote:
>
> > > + struct resource *res;
> > > + struct resource *dma_res;
> > > + struct clk *clk;
> >
> > are visible in the parent.
> >
> It seems this is a generally used approach, seen in mxcmmc and pxamci.
Just because someone else is doing it, it doesn't have to be a good
idea ;-)
But you're right, it seems to be done this way almost everywhere.
Unless Chris has a more definite opinion here, I don't mind if
you follow the same pattern.
If someone decides that it's really a bad idea, we should change
all drivers at once.
> > > + struct mxs_dma_data dma_data;
> >
> > Why do you need host specific DMA structures? Please stick to
> > the generic dma-engine API if possible.
> >
> I'm sticking to the generic dmaengine api. The mxs dma hardware
> has separate irq line for every single dma channel, which needs to
> be told by client driver.
I'm not convinced, it still sounds like a layering violation to
have specific information about the DMA controller in the
platform data of a driver using the dma engine API.
Why can't you put the interrupt number into the platform data of
the dma engine device? Your filter function already identifies
the number of the DMA channel.
> > > +static irqreturn_t mxs_mmc_irq_handler(int irq, void *dev_id)
> > > +{
> > > + struct mxs_mmc_host *host = dev_id;
> > > + u32 stat;
> > > +
> > > + stat = __raw_readl(host->base + HW_SSP_CTRL1);
> > > + __mxs_clrl(stat & MXS_MMC_IRQ_BITS, host->base + HW_SSP_CTRL1);
> > > +
> > > + if (host->cmd && (stat & MXS_MMC_ERR_BITS))
> > > + host->status = __raw_readl(host->base + HW_SSP_STATUS);
> > > +
> > > + if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN))
> > > + mmc_signal_sdio_irq(host->mmc);
> > > +
> > > + return IRQ_HANDLED;
> > > +}
> >
> > You use spin_lock_irqsave in mxs_mmc_enable_sdio_irq, but don't
> > actually use the spinlock in the interrupt handler. This means
> > that either your interrupt handler is broken because it doesn't
> > lock, or that you don't actually need the _irqsave part.
> >
> I do not understand this one. I'm seeing mxcmmc and pxamci use
> spin_lock_irqsave/irqrestore in the similar way.
The difference is that e.g. mxcmci_irq() takes the spinlock that is
used to protect the host->use_sdio flag, serializing the the
code that initializes sdio with the code that uses it.
[Actually, mxcmci_irq() also looks wrong, because it releases the
spinlock before calling mmc_signal_sdio_irq(), so sdio may be
disabled by then, but that is a slightly different bug]
What I meant is that you take care to avoid getting into the
interrupt handler while holding the spinlock, but in the handler,
you don't check if the lock is held. It can't be correct to
serialize just half the cases.
Arnd
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-10 17:17 ` Arnd Bergmann
@ 2011-02-11 23:14 ` Shawn Guo
2011-02-11 15:51 ` Arnd Bergmann
0 siblings, 1 reply; 33+ messages in thread
From: Shawn Guo @ 2011-02-11 23:14 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Feb 10, 2011 at 06:17:41PM +0100, Arnd Bergmann wrote:
> On Friday 11 February 2011, Shawn Guo wrote:
> >
> > > > + struct mxs_dma_data dma_data;
> > >
> > > Why do you need host specific DMA structures? Please stick to
> > > the generic dma-engine API if possible.
> > >
> > I'm sticking to the generic dmaengine api. The mxs dma hardware
> > has separate irq line for every single dma channel, which needs to
> > be told by client driver.
>
> I'm not convinced, it still sounds like a layering violation to
> have specific information about the DMA controller in the
> platform data of a driver using the dma engine API.
>
It sounds like something about the dma controller, but it really
belongs to dma client device e.g. ssp here. Every single dma client
device has two dma related resources, dma channel and dma irq, which
should be defined in client device data.
> Why can't you put the interrupt number into the platform data of
> the dma engine device? Your filter function already identifies
> the number of the DMA channel.
>
We have 16 channels for dma-apbh and dma-apbx respectively. And each
channel has fixed peripheral device and irq. You think we can define
2 x 16 x (channel number + channel irq) in dma engine driver? I'm
afraid not. The channel number can be identified in filter function
because dmaengine core code and mxs dma hw are indexing the channel
in the same way, so that the sw channel id can be used to address hw
channel, otherwise we have to pass hw channel id to dma driver just
like what we do with irq.
> > > > +static irqreturn_t mxs_mmc_irq_handler(int irq, void *dev_id)
> > > > +{
> > > > + struct mxs_mmc_host *host = dev_id;
> > > > + u32 stat;
> > > > +
> > > > + stat = __raw_readl(host->base + HW_SSP_CTRL1);
> > > > + __mxs_clrl(stat & MXS_MMC_IRQ_BITS, host->base + HW_SSP_CTRL1);
> > > > +
> > > > + if (host->cmd && (stat & MXS_MMC_ERR_BITS))
> > > > + host->status = __raw_readl(host->base + HW_SSP_STATUS);
> > > > +
> > > > + if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN))
> > > > + mmc_signal_sdio_irq(host->mmc);
> > > > +
> > > > + return IRQ_HANDLED;
> > > > +}
> > >
> > > You use spin_lock_irqsave in mxs_mmc_enable_sdio_irq, but don't
> > > actually use the spinlock in the interrupt handler. This means
> > > that either your interrupt handler is broken because it doesn't
> > > lock, or that you don't actually need the _irqsave part.
> > >
> > I do not understand this one. I'm seeing mxcmmc and pxamci use
> > spin_lock_irqsave/irqrestore in the similar way.
>
> The difference is that e.g. mxcmci_irq() takes the spinlock that is
> used to protect the host->use_sdio flag, serializing the the
> code that initializes sdio with the code that uses it.
>
> [Actually, mxcmci_irq() also looks wrong, because it releases the
> spinlock before calling mmc_signal_sdio_irq(), so sdio may be
> disabled by then, but that is a slightly different bug]
>
> What I meant is that you take care to avoid getting into the
> interrupt handler while holding the spinlock, but in the handler,
> you don't check if the lock is held. It can't be correct to
> serialize just half the cases.
>
Thanks for the explanation. Please help review the fix below to see
if I understand the comment correctly.
if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN)) {
spin_lock_irqsave(&host->lock, flags);
mmc_signal_sdio_irq(host->mmc);
spin_unlock_irqrestore(&host->lock, flags);
}
Regards,
Shawn
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-11 23:14 ` Shawn Guo
@ 2011-02-11 15:51 ` Arnd Bergmann
2011-02-12 0:55 ` Shawn Guo
0 siblings, 1 reply; 33+ messages in thread
From: Arnd Bergmann @ 2011-02-11 15:51 UTC (permalink / raw)
To: linux-arm-kernel
On Saturday 12 February 2011, Shawn Guo wrote:
> On Thu, Feb 10, 2011 at 06:17:41PM +0100, Arnd Bergmann wrote:
> > On Friday 11 February 2011, Shawn Guo wrote:
> > >
> > > > > + struct mxs_dma_data dma_data;
> > > >
> > > > Why do you need host specific DMA structures? Please stick to
> > > > the generic dma-engine API if possible.
> > > >
> > > I'm sticking to the generic dmaengine api. The mxs dma hardware
> > > has separate irq line for every single dma channel, which needs to
> > > be told by client driver.
> >
> > I'm not convinced, it still sounds like a layering violation to
> > have specific information about the DMA controller in the
> > platform data of a driver using the dma engine API.
> >
> It sounds like something about the dma controller, but it really
> belongs to dma client device e.g. ssp here. Every single dma client
> device has two dma related resources, dma channel and dma irq, which
> should be defined in client device data.
That is true for the DMA controller you use, but it doesn't have
to be that way for all other DMA controllers, right? My point is
that the MMC driver should not make any assumptions about what
DMA controller is used, because a future SoC might combine it
with a different DMA controller, e.g. one that just just a single
interrupt for all channels.
> > Why can't you put the interrupt number into the platform data of
> > the dma engine device? Your filter function already identifies
> > the number of the DMA channel.
> >
> We have 16 channels for dma-apbh and dma-apbx respectively. And each
> channel has fixed peripheral device and irq. You think we can define
> 2 x 16 x (channel number + channel irq) in dma engine driver? I'm
> afraid not. The channel number can be identified in filter function
> because dmaengine core code and mxs dma hw are indexing the channel
> in the same way, so that the sw channel id can be used to address hw
> channel, otherwise we have to pass hw channel id to dma driver just
> like what we do with irq.
I don't understand. The sw channel id should be something that is
defined in an arbitrary way for each machine, and it should be the
only thing that a driver needs, right?
I have not looked much at other dmaengine drivers, but I'd be
surprised if they require the device driver to be written
for a specific implementation. If that was the case, you would
not even need a dmaengine API but could just as well write
to the DMA controller registers from the device driver directly.
> > What I meant is that you take care to avoid getting into the
> > interrupt handler while holding the spinlock, but in the handler,
> > you don't check if the lock is held. It can't be correct to
> > serialize just half the cases.
> >
> Thanks for the explanation. Please help review the fix below to see
> if I understand the comment correctly.
>
> if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN)) {
> spin_lock_irqsave(&host->lock, flags);
> mmc_signal_sdio_irq(host->mmc);
> spin_unlock_irqrestore(&host->lock, flags);
> }
This is still wrong for two reasons:
* You now don't hold the lock while reading the 'stat' variable. The point of the
lock is to make sure that sdio doesn't get turned off after reading stat but
before signaling the sdio, so you have to hold it across both.
* In an interrupt controller, you should not disable interrupts again.
It's harmless, but slow and confusing to the reader.
Arnd
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-11 15:51 ` Arnd Bergmann
@ 2011-02-12 0:55 ` Shawn Guo
2011-02-11 20:04 ` Arnd Bergmann
0 siblings, 1 reply; 33+ messages in thread
From: Shawn Guo @ 2011-02-12 0:55 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Feb 11, 2011 at 04:51:35PM +0100, Arnd Bergmann wrote:
> On Saturday 12 February 2011, Shawn Guo wrote:
> > On Thu, Feb 10, 2011 at 06:17:41PM +0100, Arnd Bergmann wrote:
> > > On Friday 11 February 2011, Shawn Guo wrote:
> > > >
> > > > > > + struct mxs_dma_data dma_data;
> > > > >
> > > > > Why do you need host specific DMA structures? Please stick to
> > > > > the generic dma-engine API if possible.
> > > > >
> > > > I'm sticking to the generic dmaengine api. The mxs dma hardware
> > > > has separate irq line for every single dma channel, which needs to
> > > > be told by client driver.
> > >
> > > I'm not convinced, it still sounds like a layering violation to
> > > have specific information about the DMA controller in the
> > > platform data of a driver using the dma engine API.
> > >
> > It sounds like something about the dma controller, but it really
> > belongs to dma client device e.g. ssp here. Every single dma client
> > device has two dma related resources, dma channel and dma irq, which
> > should be defined in client device data.
>
> That is true for the DMA controller you use, but it doesn't have
> to be that way for all other DMA controllers, right? My point is
> that the MMC driver should not make any assumptions about what
> DMA controller is used, because a future SoC might combine it
> with a different DMA controller, e.g. one that just just a single
> interrupt for all channels.
>
Your thought is right. But the fact is, from hw design point of
view, all client peripherals are coupled with mxs dma. As a real
example, gpmi (mxs nand controller) is one mxs dma client device.
When gpmi is integrated into i.MX50 to replace nfc (original i.mx
nand controller), dma-aphb has to be brought into mx50 together,
though mx50 already gets sdma there.
I tried hard to decouple client driver from mxs dmaengine driver, but
it can not be that thorough. As dma peripheral only trigger error irq,
mxs dma "pio" mode and dma irq need to be handled by client and dma
driver in a coupled way.
> > > Why can't you put the interrupt number into the platform data of
> > > the dma engine device? Your filter function already identifies
> > > the number of the DMA channel.
> > >
> > We have 16 channels for dma-apbh and dma-apbx respectively. And each
> > channel has fixed peripheral device and irq. You think we can define
> > 2 x 16 x (channel number + channel irq) in dma engine driver? I'm
> > afraid not. The channel number can be identified in filter function
> > because dmaengine core code and mxs dma hw are indexing the channel
> > in the same way, so that the sw channel id can be used to address hw
> > channel, otherwise we have to pass hw channel id to dma driver just
> > like what we do with irq.
>
> I don't understand. The sw channel id should be something that is
> defined in an arbitrary way for each machine, and it should be the
> only thing that a driver needs, right?
>
Yes, the sw channel id can be defined in an arbitrary way by
dmaengine, but in that case dmaegine can not use sw channel id to
access hw channel, because each mxs dma client device gets a specific
hw channel. Of course, we can give device an arbitrary channel with
a fixed hw channel id attached on, and use hw id to access hw channel.
But in that case, we have one more info in device data to pass besides
irq.
> I have not looked much at other dmaengine drivers, but I'd be
> surprised if they require the device driver to be written
> for a specific implementation. If that was the case, you would
> not even need a dmaengine API but could just as well write
> to the DMA controller registers from the device driver directly.
>
We need a specific implementation, but it's not so specific that we
have to access dma controller directly. Even it is, we still need
an API/interface, as there are so many client devices need to do the
same thing, right? ;)
> > > What I meant is that you take care to avoid getting into the
> > > interrupt handler while holding the spinlock, but in the handler,
> > > you don't check if the lock is held. It can't be correct to
> > > serialize just half the cases.
> > >
> > Thanks for the explanation. Please help review the fix below to see
> > if I understand the comment correctly.
> >
> > if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN)) {
> > spin_lock_irqsave(&host->lock, flags);
> > mmc_signal_sdio_irq(host->mmc);
> > spin_unlock_irqrestore(&host->lock, flags);
> > }
>
> This is still wrong for two reasons:
>
> * You now don't hold the lock while reading the 'stat' variable. The point of the
> lock is to make sure that sdio doesn't get turned off after reading stat but
> before signaling the sdio, so you have to hold it across both.
> * In an interrupt controller, you should not disable interrupts again.
> It's harmless, but slow and confusing to the reader.
>
Sorry for the dumb. So it should be something like the following?
spin_lock(&host->lock);
stat = readl(host->base + HW_SSP_CTRL1);
writel(stat & MXS_MMC_IRQ_BITS,
host->base + HW_SSP_CTRL1 + MXS_CLR_ADDR);
[...]
if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN))
mmc_signal_sdio_irq(host->mmc);
spin_unlock(&host->lock);
Regards,
Shawn
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-12 0:55 ` Shawn Guo
@ 2011-02-11 20:04 ` Arnd Bergmann
2011-02-12 14:04 ` Shawn Guo
0 siblings, 1 reply; 33+ messages in thread
From: Arnd Bergmann @ 2011-02-11 20:04 UTC (permalink / raw)
To: linux-arm-kernel
On Saturday 12 February 2011 01:55:07 Shawn Guo wrote:
> On Fri, Feb 11, 2011 at 04:51:35PM +0100, Arnd Bergmann wrote:
> > I have not looked much at other dmaengine drivers, but I'd be
> > surprised if they require the device driver to be written
> > for a specific implementation. If that was the case, you would
> > not even need a dmaengine API but could just as well write
> > to the DMA controller registers from the device driver directly.
> >
> We need a specific implementation, but it's not so specific that we
> have to access dma controller directly. Even it is, we still need
> an API/interface, as there are so many client devices need to do the
> same thing, right? ;)
I looked at all mmc drivers that use the dmaengine API:
atmel-mci.c does the same as what you propose here, while sh_mmcif.c
and tmio_mmc.c more or less do what I'm suggesting you do instead.
Looking at sh_mmcif:
host->chan_tx = dma_request_channel(mask, sh_mmcif_filter,
&pdata->dma->chan_priv_tx);
This is the only place where dma engine specific data is used
in the driver, and chan_priv_tx is part of the platform data, so the
mmc driver can simply pass it down as a void pointer without knowing
the type. The platform data as defined in the machine file ties
both the dma controller and the mmc device together, but neither
of the two drivers needs to know anything about the implementation
of the other.
> So it should be something like the following?
>
> spin_lock(&host->lock);
>
> stat = readl(host->base + HW_SSP_CTRL1);
> writel(stat & MXS_MMC_IRQ_BITS,
> host->base + HW_SSP_CTRL1 + MXS_CLR_ADDR);
>
> [...]
>
> if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN))
> mmc_signal_sdio_irq(host->mmc);
>
> spin_unlock(&host->lock);
>
Yes, this looks correct to me now.
Arnd
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-11 20:04 ` Arnd Bergmann
@ 2011-02-12 14:04 ` Shawn Guo
2011-02-12 8:59 ` Arnd Bergmann
0 siblings, 1 reply; 33+ messages in thread
From: Shawn Guo @ 2011-02-12 14:04 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Feb 11, 2011 at 09:04:23PM +0100, Arnd Bergmann wrote:
> On Saturday 12 February 2011 01:55:07 Shawn Guo wrote:
> > On Fri, Feb 11, 2011 at 04:51:35PM +0100, Arnd Bergmann wrote:
> > > I have not looked much at other dmaengine drivers, but I'd be
> > > surprised if they require the device driver to be written
> > > for a specific implementation. If that was the case, you would
> > > not even need a dmaengine API but could just as well write
> > > to the DMA controller registers from the device driver directly.
> > >
> > We need a specific implementation, but it's not so specific that we
> > have to access dma controller directly. Even it is, we still need
> > an API/interface, as there are so many client devices need to do the
> > same thing, right? ;)
>
> I looked at all mmc drivers that use the dmaengine API:
> atmel-mci.c does the same as what you propose here, while sh_mmcif.c
Actually, it's not me who propose the implementation. Sascha sent a
patch (under review) to change mxcmmc.c to use generic damengine api
several weeks ago. I followed that as the example.
> and tmio_mmc.c more or less do what I'm suggesting you do instead.
>
> Looking at sh_mmcif:
>
> host->chan_tx = dma_request_channel(mask, sh_mmcif_filter,
> &pdata->dma->chan_priv_tx);
>
>
> This is the only place where dma engine specific data is used
> in the driver, and chan_priv_tx is part of the platform data, so the
> mmc driver can simply pass it down as a void pointer without knowing
> the type. The platform data as defined in the machine file ties
> both the dma controller and the mmc device together, but neither
> of the two drivers needs to know anything about the implementation
> of the other.
>
Not really. Though mmc does not need to know anything about dma
driver, dma knows that mmc driver has to pass .slave_id via
chan->private. The snippet below is copied from shdma.
struct sh_dmae_slave_config {
unsigned int slave_id;
dma_addr_t addr;
u32 chcr;
char mid_rid;
};
---
if (chan->private) {
/* The caller is holding dma_list_mutex */
struct sh_dmae_slave *param = chan->private;
clear_bit(param->slave_id, sh_dmae_slave_used);
}
And it only works when slave_id is the first member of
sh_dmae_slave_config.
For me, it's more natural to define device's dma related things like
dma channel id and irq as its resources than platform data. So I
still maintain the current approach.
Regards,
Shawn
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-12 14:04 ` Shawn Guo
@ 2011-02-12 8:59 ` Arnd Bergmann
2011-02-12 17:23 ` Shawn Guo
0 siblings, 1 reply; 33+ messages in thread
From: Arnd Bergmann @ 2011-02-12 8:59 UTC (permalink / raw)
To: linux-arm-kernel
On Saturday 12 February 2011 15:04:19 Shawn Guo wrote:
> > and tmio_mmc.c more or less do what I'm suggesting you do instead.
> >
> > Looking at sh_mmcif:
> >
> > host->chan_tx = dma_request_channel(mask, sh_mmcif_filter,
> > &pdata->dma->chan_priv_tx);
> >
> >
> > This is the only place where dma engine specific data is used
> > in the driver, and chan_priv_tx is part of the platform data, so the
> > mmc driver can simply pass it down as a void pointer without knowing
> > the type. The platform data as defined in the machine file ties
> > both the dma controller and the mmc device together, but neither
> > of the two drivers needs to know anything about the implementation
> > of the other.
> >
> Not really. Though mmc does not need to know anything about dma
> driver, dma knows that mmc driver has to pass .slave_id via
> chan->private. The snippet below is copied from shdma.
>
> struct sh_dmae_slave_config {
> unsigned int slave_id;
> dma_addr_t addr;
> u32 chcr;
> char mid_rid;
> };
> ---
> if (chan->private) {
> /* The caller is holding dma_list_mutex */
> struct sh_dmae_slave *param = chan->private;
> clear_bit(param->slave_id, sh_dmae_slave_used);
> }
>
> And it only works when slave_id is the first member of
> sh_dmae_slave_config.
>
> For me, it's more natural to define device's dma related things like
> dma channel id and irq as its resources than platform data. So I
> still maintain the current approach.
I'm not arguing against passing the data as platform data (well, not any
more, but it was never the main objection anyway).
You are right that sh_dmae_slave_config contains all the private data
for the DMA controller, and I have no objection to you doing the
same. However, sh_mmcif.c does not know the defintion of
sh_dmae_slave_config, it just gets it via a void pointer from the
platform data and passes it to the dma_request_channel function via
another void pointer. That function calls into the sh_dmae driver, which
casts it back to struct sh_dmae_slave_config, and that is same that I'm
suggesting you to do.
It should really be a trivial change to move your struct mxs_dma_data
from struct mxs_mmc_host to your platform_data:
struct mxs_mmc_platform_data {
int wp_gpio; /* write protect pin */
void *dma_data;
};
static struct mxs_dma_data mxs_mmc_dma_data = {
.chan_irq = MMC_DMA_IRQ,
};
static struct mxs_mmc_platform_data mxs_mmc_pdata = {
.wp_gpio = SOME_CONSTANT,
.dma_data = &mxs_mmc_dma_data,
}
Now only the platform definition needs to know about both
mxs_mmc_platform_data and mxs_dma_data, while the dma engine driver
and the mmc driver each just need to know about their own data.
Sorry for being too vague in what I asking for before.
Arnd
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-12 8:59 ` Arnd Bergmann
@ 2011-02-12 17:23 ` Shawn Guo
2011-02-12 10:07 ` Arnd Bergmann
0 siblings, 1 reply; 33+ messages in thread
From: Shawn Guo @ 2011-02-12 17:23 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Feb 12, 2011 at 09:59:22AM +0100, Arnd Bergmann wrote:
> On Saturday 12 February 2011 15:04:19 Shawn Guo wrote:
> > > and tmio_mmc.c more or less do what I'm suggesting you do instead.
> > >
> > > Looking at sh_mmcif:
> > >
> > > host->chan_tx = dma_request_channel(mask, sh_mmcif_filter,
> > > &pdata->dma->chan_priv_tx);
> > >
> > >
> > > This is the only place where dma engine specific data is used
> > > in the driver, and chan_priv_tx is part of the platform data, so the
> > > mmc driver can simply pass it down as a void pointer without knowing
> > > the type. The platform data as defined in the machine file ties
> > > both the dma controller and the mmc device together, but neither
> > > of the two drivers needs to know anything about the implementation
> > > of the other.
> > >
> > Not really. Though mmc does not need to know anything about dma
> > driver, dma knows that mmc driver has to pass .slave_id via
> > chan->private. The snippet below is copied from shdma.
> >
> > struct sh_dmae_slave_config {
> > unsigned int slave_id;
> > dma_addr_t addr;
> > u32 chcr;
> > char mid_rid;
> > };
> > ---
> > if (chan->private) {
> > /* The caller is holding dma_list_mutex */
> > struct sh_dmae_slave *param = chan->private;
> > clear_bit(param->slave_id, sh_dmae_slave_used);
> > }
> >
> > And it only works when slave_id is the first member of
> > sh_dmae_slave_config.
> >
> > For me, it's more natural to define device's dma related things like
> > dma channel id and irq as its resources than platform data. So I
> > still maintain the current approach.
>
> I'm not arguing against passing the data as platform data (well, not any
> more, but it was never the main objection anyway).
>
> You are right that sh_dmae_slave_config contains all the private data
> for the DMA controller, and I have no objection to you doing the
> same. However, sh_mmcif.c does not know the defintion of
> sh_dmae_slave_config, it just gets it via a void pointer from the
> platform data and passes it to the dma_request_channel function via
> another void pointer. That function calls into the sh_dmae driver, which
> casts it back to struct sh_dmae_slave_config, and that is same that I'm
> suggesting you to do.
>
> It should really be a trivial change to move your struct mxs_dma_data
> from struct mxs_mmc_host to your platform_data:
>
> struct mxs_mmc_platform_data {
> int wp_gpio; /* write protect pin */
> void *dma_data;
> };
>
> static struct mxs_dma_data mxs_mmc_dma_data = {
> .chan_irq = MMC_DMA_IRQ,
> };
>
> static struct mxs_mmc_platform_data mxs_mmc_pdata = {
> .wp_gpio = SOME_CONSTANT,
> .dma_data = &mxs_mmc_dma_data,
> }
>
> Now only the platform definition needs to know about both
> mxs_mmc_platform_data and mxs_dma_data, while the dma engine driver
> and the mmc driver each just need to know about their own data.
>
Well, we are removing inclusion of mach/dma.h from mmc driver, but
adding it to every mxs based machine code. This makes mmc driver
clean but machine code becomes not. For some dma client devices
coming later, the platform data could be saved at all, if they do not
have any. But with the approach you are suggesting, every single
client device will have to get platform data.
Regards,
Shawn
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-12 17:23 ` Shawn Guo
@ 2011-02-12 10:07 ` Arnd Bergmann
2011-02-12 18:37 ` Shawn Guo
0 siblings, 1 reply; 33+ messages in thread
From: Arnd Bergmann @ 2011-02-12 10:07 UTC (permalink / raw)
To: linux-arm-kernel
On Saturday 12 February 2011 18:23:58 Shawn Guo wrote:
> Well, we are removing inclusion of mach/dma.h from mmc driver, but
> adding it to every mxs based machine code. This makes mmc driver
> clean but machine code becomes not. For some dma client devices
> coming later, the platform data could be saved at all, if they do not
> have any. But with the approach you are suggesting, every single
> client device will have to get platform data.
Right, unless there is a way to encode it exlusively in the resources,
which is what I was suggesting at first: If the dma engine driver
knows about all the channels, you only need to pass the channel number.
If you use a flattened device tree, you can avoid the need for
platform data by adding the phandle of the dma engine device to
a property of the mmc driver, along with the channel number inside
of that device. I think that would be the cleanest approach, but
some people still need to be convinced that changing drivers to
use fdt data is the right direction for ARM.
Arnd
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-12 10:07 ` Arnd Bergmann
@ 2011-02-12 18:37 ` Shawn Guo
2011-02-12 17:30 ` Arnd Bergmann
0 siblings, 1 reply; 33+ messages in thread
From: Shawn Guo @ 2011-02-12 18:37 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Feb 12, 2011 at 11:07:17AM +0100, Arnd Bergmann wrote:
> On Saturday 12 February 2011 18:23:58 Shawn Guo wrote:
> > Well, we are removing inclusion of mach/dma.h from mmc driver, but
> > adding it to every mxs based machine code. This makes mmc driver
> > clean but machine code becomes not. For some dma client devices
> > coming later, the platform data could be saved at all, if they do not
> > have any. But with the approach you are suggesting, every single
> > client device will have to get platform data.
>
> Right, unless there is a way to encode it exlusively in the resources,
> which is what I was suggesting at first: If the dma engine driver
> knows about all the channels, you only need to pass the channel number.
>
> If you use a flattened device tree, you can avoid the need for
> platform data by adding the phandle of the dma engine device to
> a property of the mmc driver, along with the channel number inside
> of that device. I think that would be the cleanest approach, but
> some people still need to be convinced that changing drivers to
> use fdt data is the right direction for ARM.
>
Could we keep the current the approach for now and go to the cleanest
way directly when Grant's dt patches get merged?
Regards,
Shawn
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-12 18:37 ` Shawn Guo
@ 2011-02-12 17:30 ` Arnd Bergmann
0 siblings, 0 replies; 33+ messages in thread
From: Arnd Bergmann @ 2011-02-12 17:30 UTC (permalink / raw)
To: linux-arm-kernel
On Saturday 12 February 2011 19:37:27 Shawn Guo wrote:
> > If you use a flattened device tree, you can avoid the need for
> > platform data by adding the phandle of the dma engine device to
> > a property of the mmc driver, along with the channel number inside
> > of that device. I think that would be the cleanest approach, but
> > some people still need to be convinced that changing drivers to
> > use fdt data is the right direction for ARM.
> >
> Could we keep the current the approach for now and go to the cleanest
> way directly when Grant's dt patches get merged?
Sounds good to me. If you are planning to use device trees anyway,
I don't have a problem with leaving it your way in the meantime.
Arnd
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-11 0:35 ` Shawn Guo
2011-02-10 17:17 ` Arnd Bergmann
@ 2011-02-13 17:53 ` Shawn Guo
2011-02-13 15:34 ` Arnd Bergmann
1 sibling, 1 reply; 33+ messages in thread
From: Shawn Guo @ 2011-02-13 17:53 UTC (permalink / raw)
To: linux-arm-kernel
Hi Arnd,
On Fri, Feb 11, 2011 at 08:35:34AM +0800, Shawn Guo wrote:
>
> > > + unsigned present:1;
> >
> > Your card detection by polling through this variable is
> > really bad for power management. Is there really no interrupt
> > that gets triggered when installing or removing a card?
> >
> Good point. Will try to use interrupt.
>
I'm trying to use interrupt for card detection. But unfortunately,
I got stuck for some reason.
There is a known issue that mx28 gpio interrupt from bank0 can not
work, because the pin bank0 irq number 127 was used in
get_irqnr_and_base (entry-macro.S) to tell there is no pending
interrupt any more.
The mmc0 cd pin has no problem to trigger interrupt, as it's GPIO_2_9.
But mmc1 cd pin can not, because it's GPIO_0_20.
So I probably have to stay with polling.
Regards,
Shawn
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-13 17:53 ` Shawn Guo
@ 2011-02-13 15:34 ` Arnd Bergmann
2011-02-13 23:52 ` Shawn Guo
0 siblings, 1 reply; 33+ messages in thread
From: Arnd Bergmann @ 2011-02-13 15:34 UTC (permalink / raw)
To: linux-arm-kernel
On Sunday 13 February 2011 18:53:02 Shawn Guo wrote:
> On Fri, Feb 11, 2011 at 08:35:34AM +0800, Shawn Guo wrote:
> >
> > > > + unsigned present:1;
> > >
> > > Your card detection by polling through this variable is
> > > really bad for power management. Is there really no interrupt
> > > that gets triggered when installing or removing a card?
> > >
> > Good point. Will try to use interrupt.
> >
> I'm trying to use interrupt for card detection. But unfortunately,
> I got stuck for some reason.
>
> There is a known issue that mx28 gpio interrupt from bank0 can not
> work, because the pin bank0 irq number 127 was used in
> get_irqnr_and_base (entry-macro.S) to tell there is no pending
> interrupt any more.
>
> The mmc0 cd pin has no problem to trigger interrupt, as it's GPIO_2_9.
> But mmc1 cd pin can not, because it's GPIO_0_20.
>
> So I probably have to stay with polling.
I'm not sure if I understand completely. Is this a fundamental restriction,
or something that only happens on a specific board?
Maybe you can do both polling and interrupt driven card detection in
the driver, and make it depend on a board-specific quirk. E.g. if you
pass an interrupt number in the platform data, it should just use the
interrupt, but if the interrupt is missing, you can fall back to
polling.
Arnd
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-13 15:34 ` Arnd Bergmann
@ 2011-02-13 23:52 ` Shawn Guo
2011-02-13 16:10 ` Arnd Bergmann
0 siblings, 1 reply; 33+ messages in thread
From: Shawn Guo @ 2011-02-13 23:52 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, Feb 13, 2011 at 04:34:40PM +0100, Arnd Bergmann wrote:
> On Sunday 13 February 2011 18:53:02 Shawn Guo wrote:
> > On Fri, Feb 11, 2011 at 08:35:34AM +0800, Shawn Guo wrote:
> > >
> > > > > + unsigned present:1;
> > > >
> > > > Your card detection by polling through this variable is
> > > > really bad for power management. Is there really no interrupt
> > > > that gets triggered when installing or removing a card?
> > > >
> > > Good point. Will try to use interrupt.
> > >
> > I'm trying to use interrupt for card detection. But unfortunately,
> > I got stuck for some reason.
> >
> > There is a known issue that mx28 gpio interrupt from bank0 can not
> > work, because the pin bank0 irq number 127 was used in
> > get_irqnr_and_base (entry-macro.S) to tell there is no pending
> > interrupt any more.
> >
> > The mmc0 cd pin has no problem to trigger interrupt, as it's GPIO_2_9.
> > But mmc1 cd pin can not, because it's GPIO_0_20.
> >
> > So I probably have to stay with polling.
>
> I'm not sure if I understand completely. Is this a fundamental restriction,
> or something that only happens on a specific board?
>
This is a fundamental restriction.
> Maybe you can do both polling and interrupt driven card detection in
> the driver, and make it depend on a board-specific quirk. E.g. if you
> pass an interrupt number in the platform data, it should just use the
> interrupt, but if the interrupt is missing, you can fall back to
> polling.
>
I rewriting the code to use mmc core card detect polling framework.
static int mxs_mmc_get_cd(struct mmc_host *mmc)
{
struct mxs_mmc_host *host = mmc_priv(mmc);
return !(readl(host->base + HW_SSP_STATUS) &
BM_SSP_STATUS_CARD_DETECT);
}
static const struct mmc_host_ops mxs_mmc_ops = {
...
.get_cd = mxs_mmc_get_cd,
};
static int mxs_mmc_probe(struct platform_device *pdev)
{
...
mmc_caps |= MMC_CAP_NEEDS_POLL;
...
}
It looks pretty simple and mmc core code has taken care of the case
of power management. So if this looks fine to you, I will go for it
in v2 of the patch set.
Regards,
Shawn
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-13 23:52 ` Shawn Guo
@ 2011-02-13 16:10 ` Arnd Bergmann
0 siblings, 0 replies; 33+ messages in thread
From: Arnd Bergmann @ 2011-02-13 16:10 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 14 February 2011 00:52:08 Shawn Guo wrote:
> I rewriting the code to use mmc core card detect polling framework.
>
> static int mxs_mmc_get_cd(struct mmc_host *mmc)
> {
> struct mxs_mmc_host *host = mmc_priv(mmc);
>
> return !(readl(host->base + HW_SSP_STATUS) &
> BM_SSP_STATUS_CARD_DETECT);
> }
>
> static const struct mmc_host_ops mxs_mmc_ops = {
> ...
> .get_cd = mxs_mmc_get_cd,
> };
>
> static int mxs_mmc_probe(struct platform_device *pdev)
> {
> ...
> mmc_caps |= MMC_CAP_NEEDS_POLL;
> ...
> }
>
> It looks pretty simple and mmc core code has taken care of the case
> of power management. So if this looks fine to you, I will go for it
> in v2 of the patch set.
Yes, that looks very good.
Arnd
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-05 2:18 ` [PATCH 1/7] mmc: mxs-mmc: add mmc host " Shawn Guo
2011-02-04 20:26 ` Arnd Bergmann
@ 2011-02-08 11:41 ` Lothar Waßmann
2011-02-11 22:11 ` Shawn Guo
2011-02-09 7:46 ` Lothar Waßmann
2 siblings, 1 reply; 33+ messages in thread
From: Lothar Waßmann @ 2011-02-08 11:41 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Shawn Guo writes:
[...]
> +static int mxs_mmc_remove(struct platform_device *pdev)
> +{
> + struct mmc_host *mmc = platform_get_drvdata(pdev);
> + struct mxs_mmc_host *host = mmc_priv(mmc);
> +
> + platform_set_drvdata(pdev, NULL);
> +
> + mmc_remove_host(mmc);
> +
> + del_timer(&host->timer);
> +
> + free_irq(host->irq, host);
> +
> + if (host->dmach)
> + dma_release_channel(host->dmach);
> +
> + clk_disable(host->clk);
> + clk_put(host->clk);
> +
> + iounmap(host->base);
> +
> + mmc_free_host(mmc);
> +
> + release_mem_region(host->res->start, resource_size(host->res));
>
When compiled with CONFIG_PAGE_POISON this leads to:
|mmc0: card cdef removed
|Unable to handle kernel paging request at virtual address 6b6b6b6b
|pgd = c6ea4000
|[6b6b6b6b] *pgd=00000000
|Internal error: Oops: 1 [#1] PREEMPT
|last sysfs file: /sys/module/mxs_mmc/refcnt
|Modules linked in: mxs_mmc(-) evdev nand nand_ids nand_ecc tsc2007 pca953x
|CPU: 0 Not tainted (2.6.37-karo+ #100)
|PC is at mxs_mmc_remove+0x78/0x94 [mxs_mmc]
|LR is at mark_held_locks+0x5c/0x84
|pc : [<bf03310c>] lr : [<c0071da0>] psr: 20000013
|sp : c6e33ef8 ip : 6b6b6b6b fp : be825a38
|r10: 00000000 r9 : c6e32000 r8 : c0037888
|r7 : 00591700 r6 : c78d24bc r5 : c6c6ea80 r4 : c6c6ed60
|r3 : 6b6b6b6b r2 : 00000040 r1 : c6d833d0 r0 : c043af18
|Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
|Control: 0005317f Table: 46ea4000 DAC: 00000015
|Process modprobe (pid: 1217, stack limit = 0xc6e32270)
|Stack: (0xc6e33ef8 to 0xc6e34000)
|3ee0: c78d2488 bf034100
|3f00: c78d24bc c0237034 c78d2488 c0235e68 c78d2488 bf034100 c78d24bc c0235f34
|3f20: bf034100 00000080 c045c3e8 c02351c4 bf034138 00000080 c6e33f44 c007de4c
|3f40: be82599c 5f73786d 00636d6d 00000000 c01efdf0 c6d830c0 c6d830c0 c03195ec
|3f60: 00000001 c6dddbd8 c6e33f7c c0045fc4 c6d830c0 c00377d8 00000001 00000081
|3f80: 60000010 c00720d4 be825a2c 00000000 00000001 be825a2c 005916b0 00000001
|3fa0: 00000081 c00376c0 be825a2c 005916b0 00591700 00000080 be825994 00000000
|3fc0: be825a2c 005916b0 00000001 00000081 00591700 0000c69c 005916bc be825a38
|3fe0: 00591520 be8259a0 0000a42c 402aee3c 60000010 00591700 aaaaaaaa aaaaaaaa
|[<bf03310c>] (mxs_mmc_remove+0x78/0x94 [mxs_mmc]) from [<c0237034>] (platform_drv_remove+0x18/0x1c)
|[<c0237034>] (platform_drv_remove+0x18/0x1c) from [<c0235e68>] (__device_release_driver+0x64/0xa4)
|[<c0235e68>] (__device_release_driver+0x64/0xa4) from [<c0235f34>] (driver_detach+0x8c/0xb4)
|[<c0235f34>] (driver_detach+0x8c/0xb4) from [<c02351c4>] (bus_remove_driver+0x8c/0xb4)
|[<c02351c4>] (bus_remove_driver+0x8c/0xb4) from [<c007de4c>] (sys_delete_module+0x1f4/0x260)
|[<c007de4c>] (sys_delete_module+0x1f4/0x260) from [<c00376c0>] (ret_fast_syscall+0x0/0x38)
|Code: e1a00005 eb48cd47 e5943008 e59f0014 (e8930006)
|---[ end trace bb06175839554c3b ]---
indicating a use_after_free BUG!
The struct mxs_mmc_host has been already freed here by the
preceding mmc_free_host() call. This should be:
struct resource *res = host->res;
...
mmc_free_host(mmc);
release_mem_region(res->start, resource_size(res));
Lothar Wa?mann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-08 11:41 ` Lothar Waßmann
@ 2011-02-11 22:11 ` Shawn Guo
2011-02-11 14:40 ` Lothar Waßmann
0 siblings, 1 reply; 33+ messages in thread
From: Shawn Guo @ 2011-02-11 22:11 UTC (permalink / raw)
To: linux-arm-kernel
Hi Lothar,
On Tue, Feb 08, 2011 at 12:41:20PM +0100, Lothar Wa?mann wrote:
> Hi,
>
> Shawn Guo writes:
> [...]
> > +static int mxs_mmc_remove(struct platform_device *pdev)
> > +{
> > + struct mmc_host *mmc = platform_get_drvdata(pdev);
> > + struct mxs_mmc_host *host = mmc_priv(mmc);
> > +
> > + platform_set_drvdata(pdev, NULL);
> > +
> > + mmc_remove_host(mmc);
> > +
> > + del_timer(&host->timer);
> > +
> > + free_irq(host->irq, host);
> > +
> > + if (host->dmach)
> > + dma_release_channel(host->dmach);
> > +
> > + clk_disable(host->clk);
> > + clk_put(host->clk);
> > +
> > + iounmap(host->base);
> > +
> > + mmc_free_host(mmc);
> > +
> > + release_mem_region(host->res->start, resource_size(host->res));
> >
> When compiled with CONFIG_PAGE_POISON this leads to:
> |mmc0: card cdef removed
> |Unable to handle kernel paging request at virtual address 6b6b6b6b
> |pgd = c6ea4000
> |[6b6b6b6b] *pgd=00000000
> |Internal error: Oops: 1 [#1] PREEMPT
> |last sysfs file: /sys/module/mxs_mmc/refcnt
> |Modules linked in: mxs_mmc(-) evdev nand nand_ids nand_ecc tsc2007 pca953x
> |CPU: 0 Not tainted (2.6.37-karo+ #100)
> |PC is at mxs_mmc_remove+0x78/0x94 [mxs_mmc]
> |LR is at mark_held_locks+0x5c/0x84
> |pc : [<bf03310c>] lr : [<c0071da0>] psr: 20000013
> |sp : c6e33ef8 ip : 6b6b6b6b fp : be825a38
> |r10: 00000000 r9 : c6e32000 r8 : c0037888
> |r7 : 00591700 r6 : c78d24bc r5 : c6c6ea80 r4 : c6c6ed60
> |r3 : 6b6b6b6b r2 : 00000040 r1 : c6d833d0 r0 : c043af18
> |Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> |Control: 0005317f Table: 46ea4000 DAC: 00000015
> |Process modprobe (pid: 1217, stack limit = 0xc6e32270)
> |Stack: (0xc6e33ef8 to 0xc6e34000)
> |3ee0: c78d2488 bf034100
> |3f00: c78d24bc c0237034 c78d2488 c0235e68 c78d2488 bf034100 c78d24bc c0235f34
> |3f20: bf034100 00000080 c045c3e8 c02351c4 bf034138 00000080 c6e33f44 c007de4c
> |3f40: be82599c 5f73786d 00636d6d 00000000 c01efdf0 c6d830c0 c6d830c0 c03195ec
> |3f60: 00000001 c6dddbd8 c6e33f7c c0045fc4 c6d830c0 c00377d8 00000001 00000081
> |3f80: 60000010 c00720d4 be825a2c 00000000 00000001 be825a2c 005916b0 00000001
> |3fa0: 00000081 c00376c0 be825a2c 005916b0 00591700 00000080 be825994 00000000
> |3fc0: be825a2c 005916b0 00000001 00000081 00591700 0000c69c 005916bc be825a38
> |3fe0: 00591520 be8259a0 0000a42c 402aee3c 60000010 00591700 aaaaaaaa aaaaaaaa
> |[<bf03310c>] (mxs_mmc_remove+0x78/0x94 [mxs_mmc]) from [<c0237034>] (platform_drv_remove+0x18/0x1c)
> |[<c0237034>] (platform_drv_remove+0x18/0x1c) from [<c0235e68>] (__device_release_driver+0x64/0xa4)
> |[<c0235e68>] (__device_release_driver+0x64/0xa4) from [<c0235f34>] (driver_detach+0x8c/0xb4)
> |[<c0235f34>] (driver_detach+0x8c/0xb4) from [<c02351c4>] (bus_remove_driver+0x8c/0xb4)
> |[<c02351c4>] (bus_remove_driver+0x8c/0xb4) from [<c007de4c>] (sys_delete_module+0x1f4/0x260)
> |[<c007de4c>] (sys_delete_module+0x1f4/0x260) from [<c00376c0>] (ret_fast_syscall+0x0/0x38)
> |Code: e1a00005 eb48cd47 e5943008 e59f0014 (e8930006)
> |---[ end trace bb06175839554c3b ]---
> indicating a use_after_free BUG!
Thanks for catching this.
> The struct mxs_mmc_host has been already freed here by the
> preceding mmc_free_host() call. This should be:
> struct resource *res = host->res;
> ...
> mmc_free_host(mmc);
> release_mem_region(res->start, resource_size(res));
>
How about fixing it like below?
release_mem_region(host->res->start, resource_size(host->res));
mmc_free_host(mmc);
Regards,
Shawn
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-11 22:11 ` Shawn Guo
@ 2011-02-11 14:40 ` Lothar Waßmann
2011-02-12 1:08 ` Shawn Guo
0 siblings, 1 reply; 33+ messages in thread
From: Lothar Waßmann @ 2011-02-11 14:40 UTC (permalink / raw)
To: linux-arm-kernel
Shawn Guo writes:
> Hi Lothar,
>
> On Tue, Feb 08, 2011 at 12:41:20PM +0100, Lothar Wa?mann wrote:
> > Hi,
> >
> > Shawn Guo writes:
> > [...]
> > > +static int mxs_mmc_remove(struct platform_device *pdev)
> > > +{
> > > + struct mmc_host *mmc = platform_get_drvdata(pdev);
> > > + struct mxs_mmc_host *host = mmc_priv(mmc);
> > > +
> > > + platform_set_drvdata(pdev, NULL);
> > > +
> > > + mmc_remove_host(mmc);
> > > +
> > > + del_timer(&host->timer);
> > > +
> > > + free_irq(host->irq, host);
> > > +
> > > + if (host->dmach)
> > > + dma_release_channel(host->dmach);
> > > +
> > > + clk_disable(host->clk);
> > > + clk_put(host->clk);
> > > +
> > > + iounmap(host->base);
> > > +
> > > + mmc_free_host(mmc);
> > > +
> > > + release_mem_region(host->res->start, resource_size(host->res));
> > >
> > When compiled with CONFIG_PAGE_POISON this leads to:
> > |mmc0: card cdef removed
> > |Unable to handle kernel paging request at virtual address 6b6b6b6b
> > |pgd = c6ea4000
> > |[6b6b6b6b] *pgd=00000000
> > |Internal error: Oops: 1 [#1] PREEMPT
> > |last sysfs file: /sys/module/mxs_mmc/refcnt
> > |Modules linked in: mxs_mmc(-) evdev nand nand_ids nand_ecc tsc2007 pca953x
> > |CPU: 0 Not tainted (2.6.37-karo+ #100)
> > |PC is at mxs_mmc_remove+0x78/0x94 [mxs_mmc]
> > |LR is at mark_held_locks+0x5c/0x84
> > |pc : [<bf03310c>] lr : [<c0071da0>] psr: 20000013
> > |sp : c6e33ef8 ip : 6b6b6b6b fp : be825a38
> > |r10: 00000000 r9 : c6e32000 r8 : c0037888
> > |r7 : 00591700 r6 : c78d24bc r5 : c6c6ea80 r4 : c6c6ed60
> > |r3 : 6b6b6b6b r2 : 00000040 r1 : c6d833d0 r0 : c043af18
> > |Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> > |Control: 0005317f Table: 46ea4000 DAC: 00000015
> > |Process modprobe (pid: 1217, stack limit = 0xc6e32270)
> > |Stack: (0xc6e33ef8 to 0xc6e34000)
> > |3ee0: c78d2488 bf034100
> > |3f00: c78d24bc c0237034 c78d2488 c0235e68 c78d2488 bf034100 c78d24bc c0235f34
> > |3f20: bf034100 00000080 c045c3e8 c02351c4 bf034138 00000080 c6e33f44 c007de4c
> > |3f40: be82599c 5f73786d 00636d6d 00000000 c01efdf0 c6d830c0 c6d830c0 c03195ec
> > |3f60: 00000001 c6dddbd8 c6e33f7c c0045fc4 c6d830c0 c00377d8 00000001 00000081
> > |3f80: 60000010 c00720d4 be825a2c 00000000 00000001 be825a2c 005916b0 00000001
> > |3fa0: 00000081 c00376c0 be825a2c 005916b0 00591700 00000080 be825994 00000000
> > |3fc0: be825a2c 005916b0 00000001 00000081 00591700 0000c69c 005916bc be825a38
> > |3fe0: 00591520 be8259a0 0000a42c 402aee3c 60000010 00591700 aaaaaaaa aaaaaaaa
> > |[<bf03310c>] (mxs_mmc_remove+0x78/0x94 [mxs_mmc]) from [<c0237034>] (platform_drv_remove+0x18/0x1c)
> > |[<c0237034>] (platform_drv_remove+0x18/0x1c) from [<c0235e68>] (__device_release_driver+0x64/0xa4)
> > |[<c0235e68>] (__device_release_driver+0x64/0xa4) from [<c0235f34>] (driver_detach+0x8c/0xb4)
> > |[<c0235f34>] (driver_detach+0x8c/0xb4) from [<c02351c4>] (bus_remove_driver+0x8c/0xb4)
> > |[<c02351c4>] (bus_remove_driver+0x8c/0xb4) from [<c007de4c>] (sys_delete_module+0x1f4/0x260)
> > |[<c007de4c>] (sys_delete_module+0x1f4/0x260) from [<c00376c0>] (ret_fast_syscall+0x0/0x38)
> > |Code: e1a00005 eb48cd47 e5943008 e59f0014 (e8930006)
> > |---[ end trace bb06175839554c3b ]---
> > indicating a use_after_free BUG!
>
> Thanks for catching this.
>
> > The struct mxs_mmc_host has been already freed here by the
> > preceding mmc_free_host() call. This should be:
> > struct resource *res = host->res;
> > ...
> > mmc_free_host(mmc);
> > release_mem_region(res->start, resource_size(res));
> >
> How about fixing it like below?
>
> release_mem_region(host->res->start, resource_size(host->res));
> mmc_free_host(mmc);
>
It's also OK. Although I prefer to do the release_mem_region() as the
last action, because the corresponding request_mem_region() is the
first action of the driver.
I still have some more comments:
|static int mxs_mmc_remove(struct platform_device *pdev)
|{
| struct mmc_host *mmc = platform_get_drvdata(pdev);
| struct mxs_mmc_host *host = mmc_priv(mmc);
|
| platform_set_drvdata(pdev, NULL);
This should be done after the driver has been quiesced (i.e. after
free_irq). It's not relevant in this case right now, but cleaner
anyway since some timer or IRQ handler might still be triggered and
call platform_get_drvdata().
If you always care to do the actions in the remove() function in the
opposite order as the corresponding actions in the probe() function
things will usually be in the right order automatically.
|static int mxs_mmc_suspend(struct device *dev)
|{
| struct mmc_host *mmc = dev_get_drvdata(dev);
| struct mxs_mmc_host *host = mmc_priv(mmc);
| int ret = 0;
|
| if (mmc)
'mmc' cannot be NULL here. And as it has already been dereferenced
by mmc_priv(mmc) above, it makes even less sense to check it here.
|static int mxs_mmc_resume(struct device *dev)
|{
| struct mmc_host *mmc = dev_get_drvdata(dev);
| struct mxs_mmc_host *host = mmc_priv(mmc);
| int ret = 0;
|
| clk_enable(host->clk);
|
| if (mmc)
same as above.
Lothar Wa?mann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-11 14:40 ` Lothar Waßmann
@ 2011-02-12 1:08 ` Shawn Guo
0 siblings, 0 replies; 33+ messages in thread
From: Shawn Guo @ 2011-02-12 1:08 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Feb 11, 2011 at 03:40:34PM +0100, Lothar Wa?mann wrote:
> Shawn Guo writes:
> > Hi Lothar,
> >
> > On Tue, Feb 08, 2011 at 12:41:20PM +0100, Lothar Wa?mann wrote:
> > > Hi,
> > >
> > > Shawn Guo writes:
> > > [...]
> > > > +static int mxs_mmc_remove(struct platform_device *pdev)
> > > > +{
> > > > + struct mmc_host *mmc = platform_get_drvdata(pdev);
> > > > + struct mxs_mmc_host *host = mmc_priv(mmc);
> > > > +
> > > > + platform_set_drvdata(pdev, NULL);
> > > > +
> > > > + mmc_remove_host(mmc);
> > > > +
> > > > + del_timer(&host->timer);
> > > > +
> > > > + free_irq(host->irq, host);
> > > > +
> > > > + if (host->dmach)
> > > > + dma_release_channel(host->dmach);
> > > > +
> > > > + clk_disable(host->clk);
> > > > + clk_put(host->clk);
> > > > +
> > > > + iounmap(host->base);
> > > > +
> > > > + mmc_free_host(mmc);
> > > > +
> > > > + release_mem_region(host->res->start, resource_size(host->res));
> > > >
> > > When compiled with CONFIG_PAGE_POISON this leads to:
> > > |mmc0: card cdef removed
> > > |Unable to handle kernel paging request at virtual address 6b6b6b6b
> > > |pgd = c6ea4000
> > > |[6b6b6b6b] *pgd=00000000
> > > |Internal error: Oops: 1 [#1] PREEMPT
> > > |last sysfs file: /sys/module/mxs_mmc/refcnt
> > > |Modules linked in: mxs_mmc(-) evdev nand nand_ids nand_ecc tsc2007 pca953x
> > > |CPU: 0 Not tainted (2.6.37-karo+ #100)
> > > |PC is at mxs_mmc_remove+0x78/0x94 [mxs_mmc]
> > > |LR is at mark_held_locks+0x5c/0x84
> > > |pc : [<bf03310c>] lr : [<c0071da0>] psr: 20000013
> > > |sp : c6e33ef8 ip : 6b6b6b6b fp : be825a38
> > > |r10: 00000000 r9 : c6e32000 r8 : c0037888
> > > |r7 : 00591700 r6 : c78d24bc r5 : c6c6ea80 r4 : c6c6ed60
> > > |r3 : 6b6b6b6b r2 : 00000040 r1 : c6d833d0 r0 : c043af18
> > > |Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> > > |Control: 0005317f Table: 46ea4000 DAC: 00000015
> > > |Process modprobe (pid: 1217, stack limit = 0xc6e32270)
> > > |Stack: (0xc6e33ef8 to 0xc6e34000)
> > > |3ee0: c78d2488 bf034100
> > > |3f00: c78d24bc c0237034 c78d2488 c0235e68 c78d2488 bf034100 c78d24bc c0235f34
> > > |3f20: bf034100 00000080 c045c3e8 c02351c4 bf034138 00000080 c6e33f44 c007de4c
> > > |3f40: be82599c 5f73786d 00636d6d 00000000 c01efdf0 c6d830c0 c6d830c0 c03195ec
> > > |3f60: 00000001 c6dddbd8 c6e33f7c c0045fc4 c6d830c0 c00377d8 00000001 00000081
> > > |3f80: 60000010 c00720d4 be825a2c 00000000 00000001 be825a2c 005916b0 00000001
> > > |3fa0: 00000081 c00376c0 be825a2c 005916b0 00591700 00000080 be825994 00000000
> > > |3fc0: be825a2c 005916b0 00000001 00000081 00591700 0000c69c 005916bc be825a38
> > > |3fe0: 00591520 be8259a0 0000a42c 402aee3c 60000010 00591700 aaaaaaaa aaaaaaaa
> > > |[<bf03310c>] (mxs_mmc_remove+0x78/0x94 [mxs_mmc]) from [<c0237034>] (platform_drv_remove+0x18/0x1c)
> > > |[<c0237034>] (platform_drv_remove+0x18/0x1c) from [<c0235e68>] (__device_release_driver+0x64/0xa4)
> > > |[<c0235e68>] (__device_release_driver+0x64/0xa4) from [<c0235f34>] (driver_detach+0x8c/0xb4)
> > > |[<c0235f34>] (driver_detach+0x8c/0xb4) from [<c02351c4>] (bus_remove_driver+0x8c/0xb4)
> > > |[<c02351c4>] (bus_remove_driver+0x8c/0xb4) from [<c007de4c>] (sys_delete_module+0x1f4/0x260)
> > > |[<c007de4c>] (sys_delete_module+0x1f4/0x260) from [<c00376c0>] (ret_fast_syscall+0x0/0x38)
> > > |Code: e1a00005 eb48cd47 e5943008 e59f0014 (e8930006)
> > > |---[ end trace bb06175839554c3b ]---
> > > indicating a use_after_free BUG!
> >
> > Thanks for catching this.
> >
> > > The struct mxs_mmc_host has been already freed here by the
> > > preceding mmc_free_host() call. This should be:
> > > struct resource *res = host->res;
> > > ...
> > > mmc_free_host(mmc);
> > > release_mem_region(res->start, resource_size(res));
> > >
> > How about fixing it like below?
> >
> > release_mem_region(host->res->start, resource_size(host->res));
> > mmc_free_host(mmc);
> >
> It's also OK. Although I prefer to do the release_mem_region() as the
> last action, because the corresponding request_mem_region() is the
> first action of the driver.
>
OK, will respect the comment.
> I still have some more comments:
> |static int mxs_mmc_remove(struct platform_device *pdev)
> |{
> | struct mmc_host *mmc = platform_get_drvdata(pdev);
> | struct mxs_mmc_host *host = mmc_priv(mmc);
> |
> | platform_set_drvdata(pdev, NULL);
> This should be done after the driver has been quiesced (i.e. after
> free_irq). It's not relevant in this case right now, but cleaner
> anyway since some timer or IRQ handler might still be triggered and
> call platform_get_drvdata().
OK.
> If you always care to do the actions in the remove() function in the
> opposite order as the corresponding actions in the probe() function
> things will usually be in the right order automatically.
>
I do care the action order in remove() vs. probe(), and that's why
I firstly call platform_set_drvdata(pdev, NULL) in remove() ;) So
should I move platform_set_drvdata(pdev, mmc) in probe() ahead of
request_irq()?
> |static int mxs_mmc_suspend(struct device *dev)
> |{
> | struct mmc_host *mmc = dev_get_drvdata(dev);
> | struct mxs_mmc_host *host = mmc_priv(mmc);
> | int ret = 0;
> |
> | if (mmc)
> 'mmc' cannot be NULL here. And as it has already been dereferenced
> by mmc_priv(mmc) above, it makes even less sense to check it here.
>
OK.
> |static int mxs_mmc_resume(struct device *dev)
> |{
> | struct mmc_host *mmc = dev_get_drvdata(dev);
> | struct mxs_mmc_host *host = mmc_priv(mmc);
> | int ret = 0;
> |
> | clk_enable(host->clk);
> |
> | if (mmc)
> same as above.
>
OK.
Regards,
Shawn
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-05 2:18 ` [PATCH 1/7] mmc: mxs-mmc: add mmc host " Shawn Guo
2011-02-04 20:26 ` Arnd Bergmann
2011-02-08 11:41 ` Lothar Waßmann
@ 2011-02-09 7:46 ` Lothar Waßmann
2011-02-11 22:08 ` Shawn Guo
2 siblings, 1 reply; 33+ messages in thread
From: Lothar Waßmann @ 2011-02-09 7:46 UTC (permalink / raw)
To: linux-arm-kernel
Hi Shawn,
Shawn Guo writes:
> This adds the mmc host driver for Freescale MXS-based SoC i.MX23/28.
> The driver calls into mxs-dma via generic dmaengine api for both pio
> and data transfer.
>
> Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> ---
> arch/arm/mach-mxs/include/mach/mmc.h | 15 +
> drivers/mmc/host/Kconfig | 9 +
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/mxs-mmc.c | 884 ++++++++++++++++++++++++++++++++++
> 4 files changed, 909 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/mach-mxs/include/mach/mmc.h
> create mode 100644 drivers/mmc/host/mxs-mmc.c
>
I've run the mmc-test kernel module with this driver on our TX28
module which fails in the following tests:
|mmc0: Test case 15. Correct xfer_size at write (start failure)...
|mmc0: Result: ERROR (-110)
|mmc0: Test case 16. Correct xfer_size at read (start failure)...
|mmc0: Result: ERROR (-110)
|mmc0: Test case 17. Correct xfer_size at write (midway failure)...
|mmc0: Result: ERROR (-110)
|mmc0: Test case 18. Correct xfer_size at read (midway failure)...
|mmc0: Result: ERROR (-110)
Could you try the test on your hardware?
Best regards,
Lothar Wa?mann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-09 7:46 ` Lothar Waßmann
@ 2011-02-11 22:08 ` Shawn Guo
2011-02-11 14:52 ` Lothar Waßmann
0 siblings, 1 reply; 33+ messages in thread
From: Shawn Guo @ 2011-02-11 22:08 UTC (permalink / raw)
To: linux-arm-kernel
Hi Lothar,
On Wed, Feb 09, 2011 at 08:46:18AM +0100, Lothar Wa?mann wrote:
> Hi Shawn,
>
> Shawn Guo writes:
> > This adds the mmc host driver for Freescale MXS-based SoC i.MX23/28.
> > The driver calls into mxs-dma via generic dmaengine api for both pio
> > and data transfer.
> >
> > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > ---
> > arch/arm/mach-mxs/include/mach/mmc.h | 15 +
> > drivers/mmc/host/Kconfig | 9 +
> > drivers/mmc/host/Makefile | 1 +
> > drivers/mmc/host/mxs-mmc.c | 884 ++++++++++++++++++++++++++++++++++
> > 4 files changed, 909 insertions(+), 0 deletions(-)
> > create mode 100644 arch/arm/mach-mxs/include/mach/mmc.h
> > create mode 100644 drivers/mmc/host/mxs-mmc.c
> >
> I've run the mmc-test kernel module with this driver on our TX28
> module which fails in the following tests:
> |mmc0: Test case 15. Correct xfer_size at write (start failure)...
> |mmc0: Result: ERROR (-110)
> |mmc0: Test case 16. Correct xfer_size at read (start failure)...
> |mmc0: Result: ERROR (-110)
> |mmc0: Test case 17. Correct xfer_size at write (midway failure)...
> |mmc0: Result: ERROR (-110)
> |mmc0: Test case 18. Correct xfer_size at read (midway failure)...
> |mmc0: Result: ERROR (-110)
>
> Could you try the test on your hardware?
>
I'm new to this test. I enabled MMC_TEST but did not see "test"
under /sys for mmc. Can you please elaborate how to launch this
test? Thanks.
Regards,
Shawn
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-11 22:08 ` Shawn Guo
@ 2011-02-11 14:52 ` Lothar Waßmann
2011-02-11 23:48 ` Shawn Guo
0 siblings, 1 reply; 33+ messages in thread
From: Lothar Waßmann @ 2011-02-11 14:52 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Shawn Guo writes:
> Hi Lothar,
>
> On Wed, Feb 09, 2011 at 08:46:18AM +0100, Lothar Wa?mann wrote:
> > Hi Shawn,
> >
> > Shawn Guo writes:
> > > This adds the mmc host driver for Freescale MXS-based SoC i.MX23/28.
> > > The driver calls into mxs-dma via generic dmaengine api for both pio
> > > and data transfer.
> > >
> > > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > > ---
> > > arch/arm/mach-mxs/include/mach/mmc.h | 15 +
> > > drivers/mmc/host/Kconfig | 9 +
> > > drivers/mmc/host/Makefile | 1 +
> > > drivers/mmc/host/mxs-mmc.c | 884 ++++++++++++++++++++++++++++++++++
> > > 4 files changed, 909 insertions(+), 0 deletions(-)
> > > create mode 100644 arch/arm/mach-mxs/include/mach/mmc.h
> > > create mode 100644 drivers/mmc/host/mxs-mmc.c
> > >
> > I've run the mmc-test kernel module with this driver on our TX28
> > module which fails in the following tests:
> > |mmc0: Test case 15. Correct xfer_size at write (start failure)...
> > |mmc0: Result: ERROR (-110)
> > |mmc0: Test case 16. Correct xfer_size at read (start failure)...
> > |mmc0: Result: ERROR (-110)
> > |mmc0: Test case 17. Correct xfer_size at write (midway failure)...
> > |mmc0: Result: ERROR (-110)
> > |mmc0: Test case 18. Correct xfer_size at read (midway failure)...
> > |mmc0: Result: ERROR (-110)
> >
> > Could you try the test on your hardware?
> >
> I'm new to this test. I enabled MMC_TEST but did not see "test"
> under /sys for mmc. Can you please elaborate how to launch this
> test? Thanks.
>
You need debugfs mounted and the mmc-test module loaded.
An inserted card will normally be grabbed by the mmc-block driver. You
can 'unbind' the driver and 'bind' it to the mmc-test driver using the
corresponding entries in sysfs. The mmc-test driver can be controlled
via debugfs.
The following script steals an inserted card from the mmc-block driver
and performs all mmc-test testcases (TRASHING THE CONTENTS OF THE
CARD!).
<--------
#!/bin/bash
drv_path=/sys/bus/mmc/drivers
dbg_path=/sys/kernel/debug
testdrv=mmc_test
shopt -s nullglob
card=""
cleanup() {
if [ -n "$card" -a -n "$driver" -a "$driver" != "$testdrv" ];then
echo "Restoring driver $driver"
echo "$card" > "$drv_path/$testdrv/unbind"
echo "$card" > "$drv_path/$driver/bind"
fi
}
trap cleanup 0
set -e
cd "$drv_path"
for d in *;do
for l in "$d/"*;do
[ -h "$l" ] || continue
card="$(basename "$l")"
driver="$(dirname "$l")"
break
done
[ -n "$card" ] && break
done
if [ -z "$card" ];then
echo "No MMC/SD card found"
exit 1
fi
if [ "$driver" != "$testdrv" ];then
echo "Trying to claim card $card from driver $driver"
echo "$card" > "$drv_path/$driver/unbind"
echo "$card" > "$drv_path/$testdrv/bind"
fi
echo "WARNING: Card $card will be overwritten!"
echo "Hit <ENTER> to continue; <CTRL-C> to abort"
read
# perform all testcases (> 0 for individual tests)
echo 0 > "$dbg_path/${card%:*}/$card/test"
>--------
Lothar Wa?mann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
2011-02-11 14:52 ` Lothar Waßmann
@ 2011-02-11 23:48 ` Shawn Guo
0 siblings, 0 replies; 33+ messages in thread
From: Shawn Guo @ 2011-02-11 23:48 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Feb 11, 2011 at 03:52:52PM +0100, Lothar Wa?mann wrote:
> Hi,
>
> Shawn Guo writes:
> > Hi Lothar,
> >
> > On Wed, Feb 09, 2011 at 08:46:18AM +0100, Lothar Wa?mann wrote:
> > > Hi Shawn,
> > >
> > > Shawn Guo writes:
> > > > This adds the mmc host driver for Freescale MXS-based SoC i.MX23/28.
> > > > The driver calls into mxs-dma via generic dmaengine api for both pio
> > > > and data transfer.
> > > >
> > > > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > > > ---
> > > > arch/arm/mach-mxs/include/mach/mmc.h | 15 +
> > > > drivers/mmc/host/Kconfig | 9 +
> > > > drivers/mmc/host/Makefile | 1 +
> > > > drivers/mmc/host/mxs-mmc.c | 884 ++++++++++++++++++++++++++++++++++
> > > > 4 files changed, 909 insertions(+), 0 deletions(-)
> > > > create mode 100644 arch/arm/mach-mxs/include/mach/mmc.h
> > > > create mode 100644 drivers/mmc/host/mxs-mmc.c
> > > >
> > > I've run the mmc-test kernel module with this driver on our TX28
> > > module which fails in the following tests:
> > > |mmc0: Test case 15. Correct xfer_size at write (start failure)...
> > > |mmc0: Result: ERROR (-110)
> > > |mmc0: Test case 16. Correct xfer_size at read (start failure)...
> > > |mmc0: Result: ERROR (-110)
> > > |mmc0: Test case 17. Correct xfer_size at write (midway failure)...
> > > |mmc0: Result: ERROR (-110)
> > > |mmc0: Test case 18. Correct xfer_size at read (midway failure)...
> > > |mmc0: Result: ERROR (-110)
> > >
> > > Could you try the test on your hardware?
> > >
> > I'm new to this test. I enabled MMC_TEST but did not see "test"
> > under /sys for mmc. Can you please elaborate how to launch this
> > test? Thanks.
> >
> You need debugfs mounted and the mmc-test module loaded.
> An inserted card will normally be grabbed by the mmc-block driver. You
> can 'unbind' the driver and 'bind' it to the mmc-test driver using the
> corresponding entries in sysfs. The mmc-test driver can be controlled
> via debugfs.
>
> The following script steals an inserted card from the mmc-block driver
> and performs all mmc-test testcases (TRASHING THE CONTENTS OF THE
> CARD!).
> <--------
> #!/bin/bash
> drv_path=/sys/bus/mmc/drivers
> dbg_path=/sys/kernel/debug
> testdrv=mmc_test
>
> shopt -s nullglob
> card=""
>
> cleanup() {
> if [ -n "$card" -a -n "$driver" -a "$driver" != "$testdrv" ];then
> echo "Restoring driver $driver"
> echo "$card" > "$drv_path/$testdrv/unbind"
> echo "$card" > "$drv_path/$driver/bind"
> fi
> }
>
> trap cleanup 0
> set -e
> cd "$drv_path"
> for d in *;do
> for l in "$d/"*;do
> [ -h "$l" ] || continue
> card="$(basename "$l")"
> driver="$(dirname "$l")"
> break
> done
> [ -n "$card" ] && break
> done
> if [ -z "$card" ];then
> echo "No MMC/SD card found"
> exit 1
> fi
> if [ "$driver" != "$testdrv" ];then
> echo "Trying to claim card $card from driver $driver"
> echo "$card" > "$drv_path/$driver/unbind"
> echo "$card" > "$drv_path/$testdrv/bind"
> fi
> echo "WARNING: Card $card will be overwritten!"
> echo "Hit <ENTER> to continue; <CTRL-C> to abort"
> read
> # perform all testcases (> 0 for individual tests)
> echo 0 > "$dbg_path/${card%:*}/$card/test"
> >--------
>
Thanks a lot for the test script. Yes, I'm seeing the same problem
here. I will try to get it fixed in v2.
Regards,
Shawn
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 2/7] ARM: mxs/clock: fix base address missing in name##_set_parent
2011-02-05 2:18 [PATCH 0/7] Add mmc driver for i.MX23/28 Shawn Guo
2011-02-05 2:18 ` [PATCH 1/7] mmc: mxs-mmc: add mmc host " Shawn Guo
@ 2011-02-05 2:18 ` Shawn Guo
2011-02-05 2:18 ` [PATCH 3/7] ARM: mxs: make ssp error irq definition consistent Shawn Guo
` (4 subsequent siblings)
6 siblings, 0 replies; 33+ messages in thread
From: Shawn Guo @ 2011-02-05 2:18 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
---
arch/arm/mach-mxs/clock-mx23.c | 2 +-
arch/arm/mach-mxs/clock-mx28.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-mxs/clock-mx23.c b/arch/arm/mach-mxs/clock-mx23.c
index 350b28c..c19b69a 100644
--- a/arch/arm/mach-mxs/clock-mx23.c
+++ b/arch/arm/mach-mxs/clock-mx23.c
@@ -347,7 +347,7 @@ static int name##_set_parent(struct clk *clk, struct clk *parent) \
{ \
if (parent != clk->parent) { \
__raw_writel(BM_CLKCTRL_CLKSEQ_BYPASS_##bit, \
- HW_CLKCTRL_CLKSEQ_TOG); \
+ CLKCTRL_BASE_ADDR + HW_CLKCTRL_CLKSEQ_TOG); \
clk->parent = parent; \
} \
\
diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c
index a3b4787..9f4ee36 100644
--- a/arch/arm/mach-mxs/clock-mx28.c
+++ b/arch/arm/mach-mxs/clock-mx28.c
@@ -483,7 +483,7 @@ static int name##_set_parent(struct clk *clk, struct clk *parent) \
{ \
if (parent != clk->parent) { \
__raw_writel(BM_CLKCTRL_CLKSEQ_BYPASS_##bit, \
- HW_CLKCTRL_CLKSEQ_TOG); \
+ CLKCTRL_BASE_ADDR + HW_CLKCTRL_CLKSEQ_TOG); \
clk->parent = parent; \
} \
\
--
1.7.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 3/7] ARM: mxs: make ssp error irq definition consistent
2011-02-05 2:18 [PATCH 0/7] Add mmc driver for i.MX23/28 Shawn Guo
2011-02-05 2:18 ` [PATCH 1/7] mmc: mxs-mmc: add mmc host " Shawn Guo
2011-02-05 2:18 ` [PATCH 2/7] ARM: mxs/clock: fix base address missing in name##_set_parent Shawn Guo
@ 2011-02-05 2:18 ` Shawn Guo
2011-02-05 2:18 ` [PATCH 4/7] ARM: mxs: dynamically allocate mmc device Shawn Guo
` (3 subsequent siblings)
6 siblings, 0 replies; 33+ messages in thread
From: Shawn Guo @ 2011-02-05 2:18 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
---
arch/arm/mach-mxs/include/mach/mx23.h | 2 +-
arch/arm/mach-mxs/include/mach/mx28.h | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-mxs/include/mach/mx23.h b/arch/arm/mach-mxs/include/mach/mx23.h
index 1730c9c..e858692 100644
--- a/arch/arm/mach-mxs/include/mach/mx23.h
+++ b/arch/arm/mach-mxs/include/mach/mx23.h
@@ -93,7 +93,7 @@
#define MX23_INT_USB_WAKEUP 12
#define MX23_INT_GPMI_DMA 13
#define MX23_INT_SSP1_DMA 14
-#define MX23_INT_SSP_ERROR 15
+#define MX23_INT_SSP1_ERROR 15
#define MX23_INT_GPIO0 16
#define MX23_INT_GPIO1 17
#define MX23_INT_GPIO2 18
diff --git a/arch/arm/mach-mxs/include/mach/mx28.h b/arch/arm/mach-mxs/include/mach/mx28.h
index 3f3485a..75d8611 100644
--- a/arch/arm/mach-mxs/include/mach/mx28.h
+++ b/arch/arm/mach-mxs/include/mach/mx28.h
@@ -163,10 +163,10 @@
#define MX28_INT_USB0 93
#define MX28_INT_USB1_WAKEUP 94
#define MX28_INT_USB0_WAKEUP 95
-#define MX28_INT_SSP0 96
-#define MX28_INT_SSP1 97
-#define MX28_INT_SSP2 98
-#define MX28_INT_SSP3 99
+#define MX28_INT_SSP0_ERROR 96
+#define MX28_INT_SSP1_ERROR 97
+#define MX28_INT_SSP2_ERROR 98
+#define MX28_INT_SSP3_ERROR 99
#define MX28_INT_ENET_SWI 100
#define MX28_INT_ENET_MAC0 101
#define MX28_INT_ENET_MAC1 102
--
1.7.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 4/7] ARM: mxs: dynamically allocate mmc device
2011-02-05 2:18 [PATCH 0/7] Add mmc driver for i.MX23/28 Shawn Guo
` (2 preceding siblings ...)
2011-02-05 2:18 ` [PATCH 3/7] ARM: mxs: make ssp error irq definition consistent Shawn Guo
@ 2011-02-05 2:18 ` Shawn Guo
2011-02-05 2:18 ` [PATCH 5/7] ARM: mxs: fix typo "GPO" in iomux-mx23.h Shawn Guo
` (2 subsequent siblings)
6 siblings, 0 replies; 33+ messages in thread
From: Shawn Guo @ 2011-02-05 2:18 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
---
arch/arm/mach-mxs/clock-mx23.c | 16 +++++
arch/arm/mach-mxs/clock-mx28.c | 18 +++++
arch/arm/mach-mxs/devices-mx23.h | 4 +
arch/arm/mach-mxs/devices-mx28.h | 4 +
arch/arm/mach-mxs/devices/Kconfig | 3 +
arch/arm/mach-mxs/devices/Makefile | 1 +
arch/arm/mach-mxs/devices/platform-mmc.c | 80 +++++++++++++++++++++++
arch/arm/mach-mxs/include/mach/devices-common.h | 13 ++++
8 files changed, 139 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-mxs/devices/platform-mmc.c
diff --git a/arch/arm/mach-mxs/clock-mx23.c b/arch/arm/mach-mxs/clock-mx23.c
index c19b69a..a1ed5f4 100644
--- a/arch/arm/mach-mxs/clock-mx23.c
+++ b/arch/arm/mach-mxs/clock-mx23.c
@@ -445,6 +445,7 @@ static struct clk_lookup lookups[] = {
_REGISTER_CLOCK("rtc", NULL, rtc_clk)
_REGISTER_CLOCK("mxs-dma-apbh", NULL, hbus_clk)
_REGISTER_CLOCK("mxs-dma-apbx", NULL, xbus_clk)
+ _REGISTER_CLOCK("mxs-mmc.0", NULL, ssp_clk)
_REGISTER_CLOCK(NULL, "usb", usb_clk)
_REGISTER_CLOCK(NULL, "audio", audio_clk)
_REGISTER_CLOCK(NULL, "pwm", pwm_clk)
@@ -515,6 +516,15 @@ static int clk_misc_init(void)
__raw_writel(BM_CLKCTRL_CPU_INTERRUPT_WAIT,
CLKCTRL_BASE_ADDR + HW_CLKCTRL_CPU_SET);
+ /*
+ * 480 MHz seems too high to be ssp clock source directly,
+ * so set frac to get a 288 MHz ref_io.
+ */
+ reg = __raw_readl(CLKCTRL_BASE_ADDR + HW_CLKCTRL_FRAC);
+ reg &= ~BM_CLKCTRL_FRAC_IOFRAC;
+ reg |= 30 << BP_CLKCTRL_FRAC_IOFRAC;
+ __raw_writel(reg, CLKCTRL_BASE_ADDR + HW_CLKCTRL_FRAC);
+
return 0;
}
@@ -522,6 +532,12 @@ int __init mx23_clocks_init(void)
{
clk_misc_init();
+ /*
+ * source ssp clock from ref_io than ref_xtal,
+ * as ref_xtal only provides 24 MHz as maximum.
+ */
+ clk_set_parent(&ssp_clk, &ref_io_clk);
+
clk_enable(&cpu_clk);
clk_enable(&hbus_clk);
clk_enable(&xbus_clk);
diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c
index 9f4ee36..aeb7b2c 100644
--- a/arch/arm/mach-mxs/clock-mx28.c
+++ b/arch/arm/mach-mxs/clock-mx28.c
@@ -619,6 +619,8 @@ static struct clk_lookup lookups[] = {
_REGISTER_CLOCK("pll2", NULL, pll2_clk)
_REGISTER_CLOCK("mxs-dma-apbh", NULL, hbus_clk)
_REGISTER_CLOCK("mxs-dma-apbx", NULL, xbus_clk)
+ _REGISTER_CLOCK("mxs-mmc.0", NULL, ssp0_clk)
+ _REGISTER_CLOCK("mxs-mmc.1", NULL, ssp1_clk)
_REGISTER_CLOCK("flexcan.0", NULL, can0_clk)
_REGISTER_CLOCK("flexcan.1", NULL, can1_clk)
_REGISTER_CLOCK(NULL, "usb0", usb0_clk)
@@ -730,6 +732,15 @@ static int clk_misc_init(void)
reg |= BM_CLKCTRL_ENET_CLK_OUT_EN;
__raw_writel(reg, CLKCTRL_BASE_ADDR + HW_CLKCTRL_ENET);
+ /*
+ * 480 MHz seems too high to be ssp clock source directly,
+ * so set frac0 to get a 288 MHz ref_io0.
+ */
+ reg = __raw_readl(CLKCTRL_BASE_ADDR + HW_CLKCTRL_FRAC0);
+ reg &= ~BM_CLKCTRL_FRAC0_IO0FRAC;
+ reg |= 30 << BP_CLKCTRL_FRAC0_IO0FRAC;
+ __raw_writel(reg, CLKCTRL_BASE_ADDR + HW_CLKCTRL_FRAC0);
+
return 0;
}
@@ -737,6 +748,13 @@ int __init mx28_clocks_init(void)
{
clk_misc_init();
+ /*
+ * source ssp clock from ref_io0 than ref_xtal,
+ * as ref_xtal only provides 24 MHz as maximum.
+ */
+ clk_set_parent(&ssp0_clk, &ref_io0_clk);
+ clk_set_parent(&ssp1_clk, &ref_io0_clk);
+
clk_enable(&cpu_clk);
clk_enable(&hbus_clk);
clk_enable(&xbus_clk);
diff --git a/arch/arm/mach-mxs/devices-mx23.h b/arch/arm/mach-mxs/devices-mx23.h
index d5595ce..87c1cb5 100644
--- a/arch/arm/mach-mxs/devices-mx23.h
+++ b/arch/arm/mach-mxs/devices-mx23.h
@@ -20,3 +20,7 @@ extern const struct mxs_dma_data mx23_dma_data[] __initconst;
mxs_add_dma(&mx23_dma_data[0])
#define mx23_add_apbx_dma() \
mxs_add_dma(&mx23_dma_data[1])
+
+extern const struct mxs_mmc_data mx23_mmc_data[] __initconst;
+#define mx23_add_mmc(id, pdata) \
+ mxs_add_mmc(&mx23_mmc_data[id], pdata)
diff --git a/arch/arm/mach-mxs/devices-mx28.h b/arch/arm/mach-mxs/devices-mx28.h
index e3a3bbc..74db564 100644
--- a/arch/arm/mach-mxs/devices-mx28.h
+++ b/arch/arm/mach-mxs/devices-mx28.h
@@ -38,3 +38,7 @@ extern const struct mxs_flexcan_data mx28_flexcan_data[] __initconst;
mxs_add_flexcan(&mx28_flexcan_data[id], pdata)
#define mx28_add_flexcan0(pdata) mx28_add_flexcan(0, pdata)
#define mx28_add_flexcan1(pdata) mx28_add_flexcan(1, pdata)
+
+extern const struct mxs_mmc_data mx28_mmc_data[] __initconst;
+#define mx28_add_mmc(id, pdata) \
+ mxs_add_mmc(&mx28_mmc_data[id], pdata)
diff --git a/arch/arm/mach-mxs/devices/Kconfig b/arch/arm/mach-mxs/devices/Kconfig
index c2ddbc5..598a0ca 100644
--- a/arch/arm/mach-mxs/devices/Kconfig
+++ b/arch/arm/mach-mxs/devices/Kconfig
@@ -14,3 +14,6 @@ config MXS_HAVE_PLATFORM_FEC
config MXS_HAVE_PLATFORM_FLEXCAN
select HAVE_CAN_FLEXCAN if CAN
bool
+
+config MXS_HAVE_PLATFORM_MMC
+ bool
diff --git a/arch/arm/mach-mxs/devices/Makefile b/arch/arm/mach-mxs/devices/Makefile
index 510849d..0d2909a 100644
--- a/arch/arm/mach-mxs/devices/Makefile
+++ b/arch/arm/mach-mxs/devices/Makefile
@@ -3,3 +3,4 @@ obj-$(CONFIG_MXS_HAVE_PLATFORM_AUART) += platform-auart.o
obj-$(CONFIG_MXS_HAVE_PLATFORM_DMA) += platform-dma.o
obj-$(CONFIG_MXS_HAVE_PLATFORM_FEC) += platform-fec.o
obj-$(CONFIG_MXS_HAVE_PLATFORM_FLEXCAN) += platform-flexcan.o
+obj-$(CONFIG_MXS_HAVE_PLATFORM_MMC) += platform-mmc.o
diff --git a/arch/arm/mach-mxs/devices/platform-mmc.c b/arch/arm/mach-mxs/devices/platform-mmc.c
new file mode 100644
index 0000000..9065afa
--- /dev/null
+++ b/arch/arm/mach-mxs/devices/platform-mmc.c
@@ -0,0 +1,80 @@
+/*
+ * Copyright (C) 2010 Pengutronix
+ * Uwe Kleine-Koenig <u.kleine-koenig@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License version 2 as published by the
+ * Free Software Foundation.
+ */
+
+#include <linux/compiler.h>
+#include <linux/err.h>
+#include <linux/init.h>
+
+#include <mach/mx23.h>
+#include <mach/mx28.h>
+#include <mach/devices-common.h>
+
+/*
+ * mx23 reference manual is indexing all ssp resources, iomux, clock,
+ * dma channel etc from 1 than 0, while mx28 starts from 0 as normal.
+ */
+#define mx23_mmc_data_entry_single(_id) \
+ { \
+ .id = _id - 1, \
+ .iobase = MX23_SSP ## _id ## _BASE_ADDR, \
+ .dma = MX23_DMA_SSP ## _id, \
+ .irq_err = MX23_INT_SSP ## _id ## _ERROR, \
+ .irq_dma = MX23_INT_SSP ## _id ## _DMA, \
+ }
+
+#define mx28_mmc_data_entry_single(_id) \
+ { \
+ .id = _id, \
+ .iobase = MX28_SSP ## _id ## _BASE_ADDR, \
+ .dma = MX28_DMA_SSP ## _id, \
+ .irq_err = MX28_INT_SSP ## _id ## _ERROR, \
+ .irq_dma = MX28_INT_SSP ## _id ## _DMA, \
+ }
+
+#ifdef CONFIG_SOC_IMX23
+const struct mxs_mmc_data mx23_mmc_data[] __initconst = {
+ mx23_mmc_data_entry_single(1),
+ mx23_mmc_data_entry_single(2),
+};
+#endif
+
+#ifdef CONFIG_SOC_IMX28
+const struct mxs_mmc_data mx28_mmc_data[] __initconst = {
+ mx28_mmc_data_entry_single(0),
+ mx28_mmc_data_entry_single(1),
+};
+#endif
+
+struct platform_device *__init mxs_add_mmc(
+ const struct mxs_mmc_data *data,
+ const struct mxs_mmc_platform_data *pdata)
+{
+ struct resource res[] = {
+ {
+ .start = data->iobase,
+ .end = data->iobase + SZ_8K - 1,
+ .flags = IORESOURCE_MEM,
+ }, {
+ .start = data->dma,
+ .end = data->dma,
+ .flags = IORESOURCE_DMA,
+ }, {
+ .start = data->irq_err,
+ .end = data->irq_err,
+ .flags = IORESOURCE_IRQ,
+ }, {
+ .start = data->irq_dma,
+ .end = data->irq_dma,
+ .flags = IORESOURCE_IRQ,
+ },
+ };
+
+ return mxs_add_platform_device("mxs-mmc", data->id,
+ res, ARRAY_SIZE(res), pdata, sizeof(*pdata));
+}
diff --git a/arch/arm/mach-mxs/include/mach/devices-common.h b/arch/arm/mach-mxs/include/mach/devices-common.h
index acd74b4..0418e8e 100644
--- a/arch/arm/mach-mxs/include/mach/devices-common.h
+++ b/arch/arm/mach-mxs/include/mach/devices-common.h
@@ -70,3 +70,16 @@ struct mxs_flexcan_data {
struct platform_device *__init mxs_add_flexcan(
const struct mxs_flexcan_data *data,
const struct flexcan_platform_data *pdata);
+
+/* mmc */
+#include <mach/mmc.h>
+struct mxs_mmc_data {
+ int id;
+ resource_size_t iobase;
+ resource_size_t dma;
+ resource_size_t irq_err;
+ resource_size_t irq_dma;
+};
+struct platform_device *__init mxs_add_mmc(
+ const struct mxs_mmc_data *data,
+ const struct mxs_mmc_platform_data *pdata);
--
1.7.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 5/7] ARM: mxs: fix typo "GPO" in iomux-mx23.h
2011-02-05 2:18 [PATCH 0/7] Add mmc driver for i.MX23/28 Shawn Guo
` (3 preceding siblings ...)
2011-02-05 2:18 ` [PATCH 4/7] ARM: mxs: dynamically allocate mmc device Shawn Guo
@ 2011-02-05 2:18 ` Shawn Guo
2011-02-05 2:18 ` [PATCH 6/7] ARM: mxs/mx23evk: add mmc device Shawn Guo
2011-02-05 2:18 ` [PATCH 7/7] ARM: mxs/mx28evk: " Shawn Guo
6 siblings, 0 replies; 33+ messages in thread
From: Shawn Guo @ 2011-02-05 2:18 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
---
arch/arm/mach-mxs/include/mach/iomux-mx23.h | 190 +++++++++++++-------------
1 files changed, 95 insertions(+), 95 deletions(-)
diff --git a/arch/arm/mach-mxs/include/mach/iomux-mx23.h b/arch/arm/mach-mxs/include/mach/iomux-mx23.h
index 94e5dd8..b0190a4 100644
--- a/arch/arm/mach-mxs/include/mach/iomux-mx23.h
+++ b/arch/arm/mach-mxs/include/mach/iomux-mx23.h
@@ -254,102 +254,102 @@
#define MX23_PAD_ROTARYB__GPMI_CE3N MXS_IOMUX_PAD_NAKED(2, 8, PAD_MUXSEL_2)
/* MUXSEL_GPIO */
-#define MX23_PAD_GPMI_D00__GPO_0_0 MXS_IOMUX_PAD_NAKED(0, 0, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_D01__GPO_0_1 MXS_IOMUX_PAD_NAKED(0, 1, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_D02__GPO_0_2 MXS_IOMUX_PAD_NAKED(0, 2, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_D03__GPO_0_3 MXS_IOMUX_PAD_NAKED(0, 3, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_D04__GPO_0_4 MXS_IOMUX_PAD_NAKED(0, 4, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_D05__GPO_0_5 MXS_IOMUX_PAD_NAKED(0, 5, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_D06__GPO_0_6 MXS_IOMUX_PAD_NAKED(0, 6, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_D07__GPO_0_7 MXS_IOMUX_PAD_NAKED(0, 7, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_D08__GPO_0_8 MXS_IOMUX_PAD_NAKED(0, 8, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_D09__GPO_0_9 MXS_IOMUX_PAD_NAKED(0, 9, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_D10__GPO_0_10 MXS_IOMUX_PAD_NAKED(0, 10, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_D11__GPO_0_11 MXS_IOMUX_PAD_NAKED(0, 11, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_D12__GPO_0_12 MXS_IOMUX_PAD_NAKED(0, 12, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_D13__GPO_0_13 MXS_IOMUX_PAD_NAKED(0, 13, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_D14__GPO_0_14 MXS_IOMUX_PAD_NAKED(0, 14, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_D15__GPO_0_15 MXS_IOMUX_PAD_NAKED(0, 15, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_CLE__GPO_0_16 MXS_IOMUX_PAD_NAKED(0, 16, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_ALE__GPO_0_17 MXS_IOMUX_PAD_NAKED(0, 17, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_CE2N__GPO_0_18 MXS_IOMUX_PAD_NAKED(0, 18, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_RDY0__GPO_0_19 MXS_IOMUX_PAD_NAKED(0, 19, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_RDY1__GPO_0_20 MXS_IOMUX_PAD_NAKED(0, 20, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_RDY2__GPO_0_21 MXS_IOMUX_PAD_NAKED(0, 21, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_RDY3__GPO_0_22 MXS_IOMUX_PAD_NAKED(0, 22, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_WPN__GPO_0_23 MXS_IOMUX_PAD_NAKED(0, 23, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_WRN__GPO_0_24 MXS_IOMUX_PAD_NAKED(0, 24, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_RDN__GPO_0_25 MXS_IOMUX_PAD_NAKED(0, 25, PAD_MUXSEL_GPIO)
-#define MX23_PAD_AUART1_CTS__GPO_0_26 MXS_IOMUX_PAD_NAKED(0, 26, PAD_MUXSEL_GPIO)
-#define MX23_PAD_AUART1_RTS__GPO_0_27 MXS_IOMUX_PAD_NAKED(0, 27, PAD_MUXSEL_GPIO)
-#define MX23_PAD_AUART1_RX__GPO_0_28 MXS_IOMUX_PAD_NAKED(0, 28, PAD_MUXSEL_GPIO)
-#define MX23_PAD_AUART1_TX__GPO_0_29 MXS_IOMUX_PAD_NAKED(0, 29, PAD_MUXSEL_GPIO)
-#define MX23_PAD_I2C_SCL__GPO_0_30 MXS_IOMUX_PAD_NAKED(0, 30, PAD_MUXSEL_GPIO)
-#define MX23_PAD_I2C_SDA__GPO_0_31 MXS_IOMUX_PAD_NAKED(0, 31, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_D00__GPIO_0_0 MXS_IOMUX_PAD_NAKED(0, 0, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_D01__GPIO_0_1 MXS_IOMUX_PAD_NAKED(0, 1, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_D02__GPIO_0_2 MXS_IOMUX_PAD_NAKED(0, 2, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_D03__GPIO_0_3 MXS_IOMUX_PAD_NAKED(0, 3, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_D04__GPIO_0_4 MXS_IOMUX_PAD_NAKED(0, 4, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_D05__GPIO_0_5 MXS_IOMUX_PAD_NAKED(0, 5, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_D06__GPIO_0_6 MXS_IOMUX_PAD_NAKED(0, 6, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_D07__GPIO_0_7 MXS_IOMUX_PAD_NAKED(0, 7, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_D08__GPIO_0_8 MXS_IOMUX_PAD_NAKED(0, 8, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_D09__GPIO_0_9 MXS_IOMUX_PAD_NAKED(0, 9, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_D10__GPIO_0_10 MXS_IOMUX_PAD_NAKED(0, 10, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_D11__GPIO_0_11 MXS_IOMUX_PAD_NAKED(0, 11, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_D12__GPIO_0_12 MXS_IOMUX_PAD_NAKED(0, 12, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_D13__GPIO_0_13 MXS_IOMUX_PAD_NAKED(0, 13, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_D14__GPIO_0_14 MXS_IOMUX_PAD_NAKED(0, 14, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_D15__GPIO_0_15 MXS_IOMUX_PAD_NAKED(0, 15, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_CLE__GPIO_0_16 MXS_IOMUX_PAD_NAKED(0, 16, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_ALE__GPIO_0_17 MXS_IOMUX_PAD_NAKED(0, 17, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_CE2N__GPIO_0_18 MXS_IOMUX_PAD_NAKED(0, 18, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_RDY0__GPIO_0_19 MXS_IOMUX_PAD_NAKED(0, 19, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_RDY1__GPIO_0_20 MXS_IOMUX_PAD_NAKED(0, 20, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_RDY2__GPIO_0_21 MXS_IOMUX_PAD_NAKED(0, 21, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_RDY3__GPIO_0_22 MXS_IOMUX_PAD_NAKED(0, 22, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_WPN__GPIO_0_23 MXS_IOMUX_PAD_NAKED(0, 23, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_WRN__GPIO_0_24 MXS_IOMUX_PAD_NAKED(0, 24, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_RDN__GPIO_0_25 MXS_IOMUX_PAD_NAKED(0, 25, PAD_MUXSEL_GPIO)
+#define MX23_PAD_AUART1_CTS__GPIO_0_26 MXS_IOMUX_PAD_NAKED(0, 26, PAD_MUXSEL_GPIO)
+#define MX23_PAD_AUART1_RTS__GPIO_0_27 MXS_IOMUX_PAD_NAKED(0, 27, PAD_MUXSEL_GPIO)
+#define MX23_PAD_AUART1_RX__GPIO_0_28 MXS_IOMUX_PAD_NAKED(0, 28, PAD_MUXSEL_GPIO)
+#define MX23_PAD_AUART1_TX__GPIO_0_29 MXS_IOMUX_PAD_NAKED(0, 29, PAD_MUXSEL_GPIO)
+#define MX23_PAD_I2C_SCL__GPIO_0_30 MXS_IOMUX_PAD_NAKED(0, 30, PAD_MUXSEL_GPIO)
+#define MX23_PAD_I2C_SDA__GPIO_0_31 MXS_IOMUX_PAD_NAKED(0, 31, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_D00__GPO_1_0 MXS_IOMUX_PAD_NAKED(1, 0, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_D01__GPO_1_1 MXS_IOMUX_PAD_NAKED(1, 1, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_D02__GPO_1_2 MXS_IOMUX_PAD_NAKED(1, 2, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_D03__GPO_1_3 MXS_IOMUX_PAD_NAKED(1, 3, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_D04__GPO_1_4 MXS_IOMUX_PAD_NAKED(1, 4, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_D05__GPO_1_5 MXS_IOMUX_PAD_NAKED(1, 5, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_D06__GPO_1_6 MXS_IOMUX_PAD_NAKED(1, 6, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_D07__GPO_1_7 MXS_IOMUX_PAD_NAKED(1, 7, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_D08__GPO_1_8 MXS_IOMUX_PAD_NAKED(1, 8, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_D09__GPO_1_9 MXS_IOMUX_PAD_NAKED(1, 9, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_D10__GPO_1_10 MXS_IOMUX_PAD_NAKED(1, 10, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_D11__GPO_1_11 MXS_IOMUX_PAD_NAKED(1, 11, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_D12__GPO_1_12 MXS_IOMUX_PAD_NAKED(1, 12, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_D13__GPO_1_13 MXS_IOMUX_PAD_NAKED(1, 13, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_D14__GPO_1_14 MXS_IOMUX_PAD_NAKED(1, 14, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_D15__GPO_1_15 MXS_IOMUX_PAD_NAKED(1, 15, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_D16__GPO_1_16 MXS_IOMUX_PAD_NAKED(1, 16, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_D17__GPO_1_17 MXS_IOMUX_PAD_NAKED(1, 17, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_RESET__GPO_1_18 MXS_IOMUX_PAD_NAKED(1, 18, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_RS__GPO_1_19 MXS_IOMUX_PAD_NAKED(1, 19, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_WR__GPO_1_20 MXS_IOMUX_PAD_NAKED(1, 20, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_CS__GPO_1_21 MXS_IOMUX_PAD_NAKED(1, 21, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_DOTCK__GPO_1_22 MXS_IOMUX_PAD_NAKED(1, 22, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_ENABLE__GPO_1_23 MXS_IOMUX_PAD_NAKED(1, 23, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_HSYNC__GPO_1_24 MXS_IOMUX_PAD_NAKED(1, 24, PAD_MUXSEL_GPIO)
-#define MX23_PAD_LCD_VSYNC__GPO_1_25 MXS_IOMUX_PAD_NAKED(1, 25, PAD_MUXSEL_GPIO)
-#define MX23_PAD_PWM0__GPO_1_26 MXS_IOMUX_PAD_NAKED(1, 26, PAD_MUXSEL_GPIO)
-#define MX23_PAD_PWM1__GPO_1_27 MXS_IOMUX_PAD_NAKED(1, 27, PAD_MUXSEL_GPIO)
-#define MX23_PAD_PWM2__GPO_1_28 MXS_IOMUX_PAD_NAKED(1, 28, PAD_MUXSEL_GPIO)
-#define MX23_PAD_PWM3__GPO_1_29 MXS_IOMUX_PAD_NAKED(1, 29, PAD_MUXSEL_GPIO)
-#define MX23_PAD_PWM4__GPO_1_30 MXS_IOMUX_PAD_NAKED(1, 30, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_D00__GPIO_1_0 MXS_IOMUX_PAD_NAKED(1, 0, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_D01__GPIO_1_1 MXS_IOMUX_PAD_NAKED(1, 1, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_D02__GPIO_1_2 MXS_IOMUX_PAD_NAKED(1, 2, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_D03__GPIO_1_3 MXS_IOMUX_PAD_NAKED(1, 3, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_D04__GPIO_1_4 MXS_IOMUX_PAD_NAKED(1, 4, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_D05__GPIO_1_5 MXS_IOMUX_PAD_NAKED(1, 5, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_D06__GPIO_1_6 MXS_IOMUX_PAD_NAKED(1, 6, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_D07__GPIO_1_7 MXS_IOMUX_PAD_NAKED(1, 7, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_D08__GPIO_1_8 MXS_IOMUX_PAD_NAKED(1, 8, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_D09__GPIO_1_9 MXS_IOMUX_PAD_NAKED(1, 9, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_D10__GPIO_1_10 MXS_IOMUX_PAD_NAKED(1, 10, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_D11__GPIO_1_11 MXS_IOMUX_PAD_NAKED(1, 11, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_D12__GPIO_1_12 MXS_IOMUX_PAD_NAKED(1, 12, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_D13__GPIO_1_13 MXS_IOMUX_PAD_NAKED(1, 13, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_D14__GPIO_1_14 MXS_IOMUX_PAD_NAKED(1, 14, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_D15__GPIO_1_15 MXS_IOMUX_PAD_NAKED(1, 15, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_D16__GPIO_1_16 MXS_IOMUX_PAD_NAKED(1, 16, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_D17__GPIO_1_17 MXS_IOMUX_PAD_NAKED(1, 17, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_RESET__GPIO_1_18 MXS_IOMUX_PAD_NAKED(1, 18, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_RS__GPIO_1_19 MXS_IOMUX_PAD_NAKED(1, 19, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_WR__GPIO_1_20 MXS_IOMUX_PAD_NAKED(1, 20, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_CS__GPIO_1_21 MXS_IOMUX_PAD_NAKED(1, 21, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_DOTCK__GPIO_1_22 MXS_IOMUX_PAD_NAKED(1, 22, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_ENABLE__GPIO_1_23 MXS_IOMUX_PAD_NAKED(1, 23, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_HSYNC__GPIO_1_24 MXS_IOMUX_PAD_NAKED(1, 24, PAD_MUXSEL_GPIO)
+#define MX23_PAD_LCD_VSYNC__GPIO_1_25 MXS_IOMUX_PAD_NAKED(1, 25, PAD_MUXSEL_GPIO)
+#define MX23_PAD_PWM0__GPIO_1_26 MXS_IOMUX_PAD_NAKED(1, 26, PAD_MUXSEL_GPIO)
+#define MX23_PAD_PWM1__GPIO_1_27 MXS_IOMUX_PAD_NAKED(1, 27, PAD_MUXSEL_GPIO)
+#define MX23_PAD_PWM2__GPIO_1_28 MXS_IOMUX_PAD_NAKED(1, 28, PAD_MUXSEL_GPIO)
+#define MX23_PAD_PWM3__GPIO_1_29 MXS_IOMUX_PAD_NAKED(1, 29, PAD_MUXSEL_GPIO)
+#define MX23_PAD_PWM4__GPIO_1_30 MXS_IOMUX_PAD_NAKED(1, 30, PAD_MUXSEL_GPIO)
-#define MX23_PAD_SSP1_CMD__GPO_2_0 MXS_IOMUX_PAD_NAKED(2, 0, PAD_MUXSEL_GPIO)
-#define MX23_PAD_SSP1_DETECT__GPO_2_1 MXS_IOMUX_PAD_NAKED(2, 1, PAD_MUXSEL_GPIO)
-#define MX23_PAD_SSP1_DATA0__GPO_2_2 MXS_IOMUX_PAD_NAKED(2, 2, PAD_MUXSEL_GPIO)
-#define MX23_PAD_SSP1_DATA1__GPO_2_3 MXS_IOMUX_PAD_NAKED(2, 3, PAD_MUXSEL_GPIO)
-#define MX23_PAD_SSP1_DATA2__GPO_2_4 MXS_IOMUX_PAD_NAKED(2, 4, PAD_MUXSEL_GPIO)
-#define MX23_PAD_SSP1_DATA3__GPO_2_5 MXS_IOMUX_PAD_NAKED(2, 5, PAD_MUXSEL_GPIO)
-#define MX23_PAD_SSP1_SCK__GPO_2_6 MXS_IOMUX_PAD_NAKED(2, 6, PAD_MUXSEL_GPIO)
-#define MX23_PAD_ROTARYA__GPO_2_7 MXS_IOMUX_PAD_NAKED(2, 7, PAD_MUXSEL_GPIO)
-#define MX23_PAD_ROTARYB__GPO_2_8 MXS_IOMUX_PAD_NAKED(2, 8, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_A00__GPO_2_9 MXS_IOMUX_PAD_NAKED(2, 9, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_A01__GPO_2_10 MXS_IOMUX_PAD_NAKED(2, 10, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_A02__GPO_2_11 MXS_IOMUX_PAD_NAKED(2, 11, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_A03__GPO_2_12 MXS_IOMUX_PAD_NAKED(2, 12, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_A04__GPO_2_13 MXS_IOMUX_PAD_NAKED(2, 13, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_A05__GPO_2_14 MXS_IOMUX_PAD_NAKED(2, 14, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_A06__GPO_2_15 MXS_IOMUX_PAD_NAKED(2, 15, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_A07__GPO_2_16 MXS_IOMUX_PAD_NAKED(2, 16, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_A08__GPO_2_17 MXS_IOMUX_PAD_NAKED(2, 17, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_A09__GPO_2_18 MXS_IOMUX_PAD_NAKED(2, 18, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_A10__GPO_2_19 MXS_IOMUX_PAD_NAKED(2, 19, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_A11__GPO_2_20 MXS_IOMUX_PAD_NAKED(2, 20, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_A12__GPO_2_21 MXS_IOMUX_PAD_NAKED(2, 21, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_BA0__GPO_2_22 MXS_IOMUX_PAD_NAKED(2, 22, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_BA1__GPO_2_23 MXS_IOMUX_PAD_NAKED(2, 23, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_CASN__GPO_2_24 MXS_IOMUX_PAD_NAKED(2, 24, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_CE0N__GPO_2_25 MXS_IOMUX_PAD_NAKED(2, 25, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_CE1N__GPO_2_26 MXS_IOMUX_PAD_NAKED(2, 26, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_CE1N__GPO_2_27 MXS_IOMUX_PAD_NAKED(2, 27, PAD_MUXSEL_GPIO)
-#define MX23_PAD_GPMI_CE0N__GPO_2_28 MXS_IOMUX_PAD_NAKED(2, 28, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_CKE__GPO_2_29 MXS_IOMUX_PAD_NAKED(2, 29, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_RASN__GPO_2_30 MXS_IOMUX_PAD_NAKED(2, 30, PAD_MUXSEL_GPIO)
-#define MX23_PAD_EMI_WEN__GPO_2_31 MXS_IOMUX_PAD_NAKED(2, 31, PAD_MUXSEL_GPIO)
+#define MX23_PAD_SSP1_CMD__GPIO_2_0 MXS_IOMUX_PAD_NAKED(2, 0, PAD_MUXSEL_GPIO)
+#define MX23_PAD_SSP1_DETECT__GPIO_2_1 MXS_IOMUX_PAD_NAKED(2, 1, PAD_MUXSEL_GPIO)
+#define MX23_PAD_SSP1_DATA0__GPIO_2_2 MXS_IOMUX_PAD_NAKED(2, 2, PAD_MUXSEL_GPIO)
+#define MX23_PAD_SSP1_DATA1__GPIO_2_3 MXS_IOMUX_PAD_NAKED(2, 3, PAD_MUXSEL_GPIO)
+#define MX23_PAD_SSP1_DATA2__GPIO_2_4 MXS_IOMUX_PAD_NAKED(2, 4, PAD_MUXSEL_GPIO)
+#define MX23_PAD_SSP1_DATA3__GPIO_2_5 MXS_IOMUX_PAD_NAKED(2, 5, PAD_MUXSEL_GPIO)
+#define MX23_PAD_SSP1_SCK__GPIO_2_6 MXS_IOMUX_PAD_NAKED(2, 6, PAD_MUXSEL_GPIO)
+#define MX23_PAD_ROTARYA__GPIO_2_7 MXS_IOMUX_PAD_NAKED(2, 7, PAD_MUXSEL_GPIO)
+#define MX23_PAD_ROTARYB__GPIO_2_8 MXS_IOMUX_PAD_NAKED(2, 8, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_A00__GPIO_2_9 MXS_IOMUX_PAD_NAKED(2, 9, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_A01__GPIO_2_10 MXS_IOMUX_PAD_NAKED(2, 10, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_A02__GPIO_2_11 MXS_IOMUX_PAD_NAKED(2, 11, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_A03__GPIO_2_12 MXS_IOMUX_PAD_NAKED(2, 12, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_A04__GPIO_2_13 MXS_IOMUX_PAD_NAKED(2, 13, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_A05__GPIO_2_14 MXS_IOMUX_PAD_NAKED(2, 14, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_A06__GPIO_2_15 MXS_IOMUX_PAD_NAKED(2, 15, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_A07__GPIO_2_16 MXS_IOMUX_PAD_NAKED(2, 16, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_A08__GPIO_2_17 MXS_IOMUX_PAD_NAKED(2, 17, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_A09__GPIO_2_18 MXS_IOMUX_PAD_NAKED(2, 18, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_A10__GPIO_2_19 MXS_IOMUX_PAD_NAKED(2, 19, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_A11__GPIO_2_20 MXS_IOMUX_PAD_NAKED(2, 20, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_A12__GPIO_2_21 MXS_IOMUX_PAD_NAKED(2, 21, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_BA0__GPIO_2_22 MXS_IOMUX_PAD_NAKED(2, 22, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_BA1__GPIO_2_23 MXS_IOMUX_PAD_NAKED(2, 23, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_CASN__GPIO_2_24 MXS_IOMUX_PAD_NAKED(2, 24, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_CE0N__GPIO_2_25 MXS_IOMUX_PAD_NAKED(2, 25, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_CE1N__GPIO_2_26 MXS_IOMUX_PAD_NAKED(2, 26, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_CE1N__GPIO_2_27 MXS_IOMUX_PAD_NAKED(2, 27, PAD_MUXSEL_GPIO)
+#define MX23_PAD_GPMI_CE0N__GPIO_2_28 MXS_IOMUX_PAD_NAKED(2, 28, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_CKE__GPIO_2_29 MXS_IOMUX_PAD_NAKED(2, 29, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_RASN__GPIO_2_30 MXS_IOMUX_PAD_NAKED(2, 30, PAD_MUXSEL_GPIO)
+#define MX23_PAD_EMI_WEN__GPIO_2_31 MXS_IOMUX_PAD_NAKED(2, 31, PAD_MUXSEL_GPIO)
#endif /* __MACH_IOMUX_MX23_H__ */
--
1.7.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 6/7] ARM: mxs/mx23evk: add mmc device
2011-02-05 2:18 [PATCH 0/7] Add mmc driver for i.MX23/28 Shawn Guo
` (4 preceding siblings ...)
2011-02-05 2:18 ` [PATCH 5/7] ARM: mxs: fix typo "GPO" in iomux-mx23.h Shawn Guo
@ 2011-02-05 2:18 ` Shawn Guo
2011-02-05 2:18 ` [PATCH 7/7] ARM: mxs/mx28evk: " Shawn Guo
6 siblings, 0 replies; 33+ messages in thread
From: Shawn Guo @ 2011-02-05 2:18 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
---
arch/arm/mach-mxs/Kconfig | 1 +
arch/arm/mach-mxs/mach-mx23evk.c | 58 ++++++++++++++++++++++++++++++++++++++
2 files changed, 59 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-mxs/Kconfig b/arch/arm/mach-mxs/Kconfig
index c9ac415..3f400cb 100644
--- a/arch/arm/mach-mxs/Kconfig
+++ b/arch/arm/mach-mxs/Kconfig
@@ -20,6 +20,7 @@ config MACH_MX23EVK
select SOC_IMX23
select MXS_HAVE_AMBA_DUART
select MXS_HAVE_PLATFORM_DMA
+ select MXS_HAVE_PLATFORM_MMC
default y
help
Include support for MX23EVK platform. This includes specific
diff --git a/arch/arm/mach-mxs/mach-mx23evk.c b/arch/arm/mach-mxs/mach-mx23evk.c
index 13c22a0..3e6bc46 100644
--- a/arch/arm/mach-mxs/mach-mx23evk.c
+++ b/arch/arm/mach-mxs/mach-mx23evk.c
@@ -26,10 +26,65 @@
#include "devices-mx23.h"
+#define MX23EVK_MMC1_WRITE_PROTECT MXS_GPIO_NR(1, 30)
+#define MX23EVK_MMC1_SLOT_POWER MXS_GPIO_NR(1, 29)
+
static const iomux_cfg_t mx23evk_pads[] __initconst = {
/* duart */
MX23_PAD_PWM0__DUART_RX | MXS_PAD_4MA,
MX23_PAD_PWM1__DUART_TX | MXS_PAD_4MA,
+
+ /* mmc */
+ MX23_PAD_SSP1_DATA0__SSP1_DATA0 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX23_PAD_SSP1_DATA1__SSP1_DATA1 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX23_PAD_SSP1_DATA2__SSP1_DATA2 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX23_PAD_SSP1_DATA3__SSP1_DATA3 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX23_PAD_GPMI_D08__SSP1_DATA4 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX23_PAD_GPMI_D09__SSP1_DATA5 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX23_PAD_GPMI_D10__SSP1_DATA6 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX23_PAD_GPMI_D11__SSP1_DATA7 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX23_PAD_SSP1_CMD__SSP1_CMD |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX23_PAD_SSP1_DETECT__SSP1_DETECT |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
+ MX23_PAD_SSP1_SCK__SSP1_SCK |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
+ /* write protect */
+ MX23_PAD_PWM4__GPIO_1_30 |
+ (MXS_PAD_4MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
+ /* slot power enable */
+ MX23_PAD_PWM3__GPIO_1_29 |
+ (MXS_PAD_4MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
+};
+
+/* mmc */
+static void __init mx23evk_mmc_slot_poweron(int gpio)
+{
+ int ret;
+
+ ret = gpio_request(gpio, "mmc-slot-power");
+ if (ret) {
+ pr_err("Failed to request gpio mmc-slot-power: %d\n", ret);
+ return;
+ }
+
+ ret = gpio_direction_output(gpio, 0);
+ if (ret) {
+ pr_err("Failed to drive gpio mmc-slot-power: %d\n", ret);
+ return;
+ }
+}
+
+static struct mxs_mmc_platform_data mx23_mmc_pdata __initdata = {
+ .wp_gpio = MX23EVK_MMC1_WRITE_PROTECT,
};
static void __init mx23evk_init(void)
@@ -45,6 +100,9 @@ static void __init mx23evk_init(void)
*/
mx23_add_apbh_dma();
mx23_add_apbx_dma();
+
+ mx23evk_mmc_slot_poweron(MX23EVK_MMC1_SLOT_POWER);
+ mx23_add_mmc(0, &mx23_mmc_pdata);
}
static void __init mx23evk_timer_init(void)
--
1.7.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 7/7] ARM: mxs/mx28evk: add mmc device
2011-02-05 2:18 [PATCH 0/7] Add mmc driver for i.MX23/28 Shawn Guo
` (5 preceding siblings ...)
2011-02-05 2:18 ` [PATCH 6/7] ARM: mxs/mx23evk: add mmc device Shawn Guo
@ 2011-02-05 2:18 ` Shawn Guo
6 siblings, 0 replies; 33+ messages in thread
From: Shawn Guo @ 2011-02-05 2:18 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
---
arch/arm/mach-mxs/Kconfig | 1 +
arch/arm/mach-mxs/mach-mx28evk.c | 102 +++++++++++++++++++++++++++++++++++++-
2 files changed, 101 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-mxs/Kconfig b/arch/arm/mach-mxs/Kconfig
index 3f400cb..b00dd2a 100644
--- a/arch/arm/mach-mxs/Kconfig
+++ b/arch/arm/mach-mxs/Kconfig
@@ -32,6 +32,7 @@ config MACH_MX28EVK
select MXS_HAVE_AMBA_DUART
select MXS_HAVE_PLATFORM_DMA
select MXS_HAVE_PLATFORM_FEC
+ select MXS_HAVE_PLATFORM_MMC
select MXS_OCOTP
default y
help
diff --git a/arch/arm/mach-mxs/mach-mx28evk.c b/arch/arm/mach-mxs/mach-mx28evk.c
index 04ddec1..9752ec2 100644
--- a/arch/arm/mach-mxs/mach-mx28evk.c
+++ b/arch/arm/mach-mxs/mach-mx28evk.c
@@ -28,8 +28,13 @@
#include "devices-mx28.h"
#include "gpio.h"
-#define MX28EVK_FEC_PHY_POWER MXS_GPIO_NR(2, 15)
-#define MX28EVK_FEC_PHY_RESET MXS_GPIO_NR(4, 13)
+#define MX28EVK_FEC_PHY_POWER MXS_GPIO_NR(2, 15)
+#define MX28EVK_FEC_PHY_RESET MXS_GPIO_NR(4, 13)
+
+#define MX28EVK_MMC0_WRITE_PROTECT MXS_GPIO_NR(2, 12)
+#define MX28EVK_MMC1_WRITE_PROTECT MXS_GPIO_NR(0, 28)
+#define MX28EVK_MMC0_SLOT_POWER MXS_GPIO_NR(3, 28)
+#define MX28EVK_MMC1_SLOT_POWER MXS_GPIO_NR(3, 29)
static const iomux_cfg_t mx28evk_pads[] __initconst = {
/* duart */
@@ -76,6 +81,66 @@ static const iomux_cfg_t mx28evk_pads[] __initconst = {
/* phy reset line */
MX28_PAD_ENET0_RX_CLK__GPIO_4_13 |
(MXS_PAD_4MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
+
+ /* mmc0 */
+ MX28_PAD_SSP0_DATA0__SSP0_D0 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX28_PAD_SSP0_DATA1__SSP0_D1 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX28_PAD_SSP0_DATA2__SSP0_D2 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX28_PAD_SSP0_DATA3__SSP0_D3 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX28_PAD_SSP0_DATA4__SSP0_D4 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX28_PAD_SSP0_DATA5__SSP0_D5 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX28_PAD_SSP0_DATA6__SSP0_D6 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX28_PAD_SSP0_DATA7__SSP0_D7 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX28_PAD_SSP0_CMD__SSP0_CMD |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX28_PAD_SSP0_DETECT__SSP0_CARD_DETECT |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
+ MX28_PAD_SSP0_SCK__SSP0_SCK |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
+ /* write protect */
+ MX28_PAD_SSP1_SCK__GPIO_2_12 |
+ (MXS_PAD_4MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
+ /* slot power enable */
+ MX28_PAD_PWM3__GPIO_3_28 |
+ (MXS_PAD_4MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
+
+ /* mmc1 */
+ MX28_PAD_GPMI_D00__SSP1_D0 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX28_PAD_GPMI_D01__SSP1_D1 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX28_PAD_GPMI_D02__SSP1_D2 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX28_PAD_GPMI_D03__SSP1_D3 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX28_PAD_GPMI_D04__SSP1_D4 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX28_PAD_GPMI_D05__SSP1_D5 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX28_PAD_GPMI_D06__SSP1_D6 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX28_PAD_GPMI_D07__SSP1_D7 |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX28_PAD_GPMI_RDY1__SSP1_CMD |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+ MX28_PAD_GPMI_RDY0__SSP1_CARD_DETECT |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
+ MX28_PAD_GPMI_WRN__SSP1_SCK |
+ (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
+ /* write protect */
+ MX28_PAD_GPMI_RESETN__GPIO_0_28 |
+ (MXS_PAD_4MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
+ /* slot power enable */
+ MX28_PAD_PWM4__GPIO_3_29 |
+ (MXS_PAD_4MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
};
/* fec */
@@ -159,6 +224,34 @@ error:
return -ETIMEDOUT;
}
+/* mmc */
+static void __init mx28evk_mmc_slot_poweron(int gpio)
+{
+ int ret;
+
+ ret = gpio_request(gpio, "mmc-slot-power");
+ if (ret) {
+ pr_err("Failed to request gpio mmc-slot-power: %d\n", ret);
+ return;
+ }
+
+ ret = gpio_direction_output(gpio, 0);
+ if (ret) {
+ pr_err("Failed to drive gpio mmc-slot-power: %d\n", ret);
+ return;
+ }
+}
+
+static struct mxs_mmc_platform_data mx28_mmc_pdata[] __initdata = {
+ {
+ /* mmc0 */
+ .wp_gpio = MX28EVK_MMC0_WRITE_PROTECT,
+ }, {
+ /* mmc1 */
+ .wp_gpio = MX28EVK_MMC1_WRITE_PROTECT,
+ },
+};
+
static void __init mx28evk_init(void)
{
mxs_iomux_setup_multiple_pads(mx28evk_pads, ARRAY_SIZE(mx28evk_pads));
@@ -173,6 +266,11 @@ static void __init mx28evk_init(void)
mx28_add_apbh_dma();
mx28_add_apbx_dma();
+ mx28evk_mmc_slot_poweron(MX28EVK_MMC0_SLOT_POWER);
+ mx28_add_mmc(0, &mx28_mmc_pdata[0]);
+ mx28evk_mmc_slot_poweron(MX28EVK_MMC1_SLOT_POWER);
+ mx28_add_mmc(1, &mx28_mmc_pdata[0]);
+
if (mx28evk_fec_get_mac())
pr_warn("%s: failed on fec mac setup\n", __func__);
--
1.7.1
^ permalink raw reply related [flat|nested] 33+ messages in thread