Hi Wolfram: See my comments below. Marked by [<Zhu Richard-r65037>] Best Regards, Richard Zhu Freescale Semiconductor Tel: +86-021-28937189 Email:Hong-Xing.Zhu@xxxxxxxxxxxxx -----Original Message----- From: Wolfram Sang [mailto:w.sang@xxxxxxxxxxxxxx] Sent: Monday, 6 September, 2010 18:47 To: Zhu Richard-R65037 Cc: linux-mmc@xxxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx Subject: Re: [PATCH 8/9] esdhc-4 esdhc: fsl esdhc driver based on platform sdhci api On Wed, Sep 01, 2010 at 05:48:53PM +0800, Richard Zhu wrote: > Based on SDHCI platform API, PIO and simple internal DMA are enabled. > > Signed-off-by: Richard Zhu <r65037@xxxxxxxxxxxxx> > --- > drivers/mmc/host/Kconfig | 12 + > drivers/mmc/host/Makefile | 1 + > drivers/mmc/host/sdhci-fsl.c | 481 > ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 494 insertions(+), 0 deletions(-) create mode > 100644 drivers/mmc/host/sdhci-fsl.c > > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig index > 283190b..2b67e11 100644 > --- a/drivers/mmc/host/Kconfig > +++ b/drivers/mmc/host/Kconfig > @@ -167,6 +167,18 @@ config MMC_SDHCI_S3C_DMA > > YMMV. > > +config MMC_SDHCI_FSL > + tristate "Freescale i.MX platform eSDHCI support" > + depends on ARCH_MX5 && MMC_SDHCI > + select MMC_SDHCI_IO_ACCESSORS > + help > + This selects the Freescale i.MX Enhanced Secure Card Host > + Controller Interface. > + If you have a i.MX platform with a Multimedia Card slot, > + say Y or M here. > + > + If unsure, say N. > + > config MMC_OMAP > tristate "TI OMAP Multimedia Card Interface support" > depends on ARCH_OMAP > diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile > index 840bcb5..649656c 100644 > --- a/drivers/mmc/host/Makefile > +++ b/drivers/mmc/host/Makefile > @@ -13,6 +13,7 @@ obj-$(CONFIG_MMC_MXC) += mxcmmc.o > obj-$(CONFIG_MMC_SDHCI) += sdhci.o > obj-$(CONFIG_MMC_SDHCI_PCI) += sdhci-pci.o > obj-$(CONFIG_MMC_SDHCI_S3C) += sdhci-s3c.o > +obj-$(CONFIG_MMC_SDHCI_FSL) += sdhci-fsl.o > obj-$(CONFIG_MMC_SDHCI_SPEAR) += sdhci-spear.o > obj-$(CONFIG_MMC_WBSD) += wbsd.o > obj-$(CONFIG_MMC_AU1X) += au1xmmc.o > diff --git a/drivers/mmc/host/sdhci-fsl.c > b/drivers/mmc/host/sdhci-fsl.c new file mode 100644 index > 0000000..fa57ec4 > --- /dev/null > +++ b/drivers/mmc/host/sdhci-fsl.c > @@ -0,0 +1,481 @@ > +/* > + * Copyright (C) 2010 Freescale Semiconductor, Inc. All Rights Reserved. > + */ > + > +/* > + * sdhci-fsl.c Support for SDHCI platform devices > + * > + * 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. > + * > + * 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., 675 Mass Ave, Cambridge, MA 02139, USA. > + */ > + > +/* Supports: > + * SDHCI fsl devices > + * > + * Inspired by sdhci-pci.c, by Pierre Ossman */ > + > +#include <linux/delay.h> > +#include <linux/highmem.h> > +#include <linux/platform_device.h> > +#include <linux/clk.h> > +#include <linux/regulator/consumer.h> #include <linux/io.h> #include > +<linux/irq.h> #include <linux/mmc/host.h> > + > +#include <mach/mmc.h> > + > +#include "sdhci.h" > + > +#define DRIVER_NAME "imx-sdhci" > + > +enum { > + /* Vendor specified registors */ > + FSL_SDHCI_WML = 0x44, > + FSL_SDHCI_WML_16_WORDS = 0x08100810, > + FSL_SDHCI_WML_32_WORDS = 0x08200820, > + FSL_SDHCI_WML_64_WORDS = 0x08400840, > + FSL_SDHCI_WML_128_WORDS = 0x08800880, > + > + /* Non-standard clock control registor */ > + FSL_SDHCI_CLOCK_MASK = 0xFFFF, > + FSL_SDHCI_CLOCK_SD_EN = 0x00000008, > + FSL_SDHCI_CLOCK_HLK_EN = 0x00000002, > + > + /* Non-standard host control registor */ > + FSL_SDHCI_CTRL_8BITBUS = 0x00000004, > +}; > + > +/********************************************************************** *******\ > + * * > + * SDHCI core callbacks * > + * * > +\******************************************************************** > +*********/ > + > +struct sdhci_fsl_host { > + struct sdhci_host *host; > + struct platform_device *pdev; /*! Platform dev */ > + struct imxmmc_platform_data *pdata; /*! private data */ > + struct clk *clk_input; /*! Clock */ > + struct regulator *regulator_mmc; /*! Regulator */ > +}; > + > +static u32 sdhci_fsl_readl(struct sdhci_host *host, int reg) { > + return readl(host->ioaddr + reg); > +} > + > +static u16 sdhci_fsl_readw(struct sdhci_host *host, int reg) { > + u32 val; > + u16 rc; > + > + if (reg % 4 == 3) { > + printk(KERN_ERR "Invalid reg address!\n"); > + return 0; > + } > + > + val = readl(host->ioaddr + (reg / 4) * 4); > + rc = (val >> (reg % 4) * 8) & 0xFFFF; > + > + return rc; > +} > + > +static u8 sdhci_fsl_readb(struct sdhci_host *host, int reg) { > + u32 val; > + u8 rc; > + > + val = readl(host->ioaddr + (reg / 4) * 4); > + rc = (val >> (reg % 4) * 8) & 0xFF; > + > + return rc; > +} > + > +static void sdhci_fsl_writel(struct sdhci_host *host, u32 val, int > +reg) { > + writel(val, host->ioaddr + reg); > +} > + > +static void sdhci_fsl_writew(struct sdhci_host *host, u16 val, int > +reg) { > + u32 io_val; > + > + if (reg % 4 == 3) { > + printk(KERN_ERR "Invalid reg address!\n"); > + return; > + } > + > + io_val = readl(host->ioaddr + (reg / 4) * 4); > + io_val &= ~(0xFFFF << ((reg % 4) * 8)); > + io_val |= val << ((reg % 4) * 8); > + > + writel(io_val, host->ioaddr + (reg / 4) * 4); } > + > +static void sdhci_fsl_writeb(struct sdhci_host *host, u8 val, int > +reg) { > + u32 io_val; > + > + io_val = readl(host->ioaddr + (reg / 4) * 4); > + io_val &= ~(0xFF << ((reg % 4) * 8)); > + io_val |= val << ((reg % 4) * 8); > + > + writel(io_val, host->ioaddr + (reg / 4) * 4); } > + > +static unsigned int sdhci_fsl_get_max_clock(struct sdhci_host *host) > +{ > + struct sdhci_fsl_host *fsl_host = sdhci_priv(host); > + > + return fsl_host->pdata->max_clk; > +} > + > +static unsigned int sdhci_fsl_get_min_clock(struct sdhci_host *host) > +{ > + struct sdhci_fsl_host *fsl_host = sdhci_priv(host); > + > + return fsl_host->pdata->min_clk; > +} > + > +static unsigned int sdhci_fsl_get_timeout_clock(struct sdhci_host > +*host) { > + return sdhci_fsl_get_max_clock(host) / 1000000; } > + > +static unsigned int sdhci_fsl_get_ro(struct sdhci_host *host) { > + struct sdhci_fsl_host *fsl_host = sdhci_priv(host); > + > + return fsl_host->pdata->wp_status(host->mmc->parent); > +} Is there really no way to route the WP-GPIO to the SD_WP pin of the controller (sigh)? Still, I'd think we can be more abstract by using the gpiolib and just pass the gpio number in the platform-data? [<Zhu Richard-r65037>] Up to now, refer to the limitations of the HW design on the different boards, the WP-GPIO mechanism is used. About the more abstract method by passing the GPIO number in the platform-data. Do you means that just pass the GPIO number in the sdhci_pltfm_data defined in sdhci_pltfm_data.h file? I don't think we can do that, because the WP-GPIO pins maybe different refer to different boards. The definition of the private platform_data should be related to the definite HW board, otherwise the abstract sdhci platform driver layer. > + > +static void sdhci_fsl_set_clock(struct sdhci_host *host, unsigned int > +clock) { > + /*This variable holds the value of clock divider, prescaler */ > + int div = 0, prescaler = 1; > + int clk_rate = 0; > + u32 clk; > + unsigned long timeout; > + struct sdhci_fsl_host *fsl_host = sdhci_priv(host); > + > + if (clock == 0) { > + host->clock = 0; > + clk = readl(host->ioaddr + SDHCI_CLOCK_CONTROL); > + writel(clk & (~FSL_SDHCI_CLOCK_HLK_EN), > + host->ioaddr + SDHCI_CLOCK_CONTROL); > + return; > + } > + > + clk_rate = clk_get_rate(fsl_host->clk_input); > + clk = readl(host->ioaddr + SDHCI_CLOCK_CONTROL) & ~FSL_SDHCI_CLOCK_MASK; > + /* precaler can't be set to zero in CLK_CTL reg */ > + clk |= (prescaler << 8); > + writel(clk, host->ioaddr + SDHCI_CLOCK_CONTROL); > + > + if (clock == sdhci_fsl_get_min_clock(host)) > + prescaler = 0x10; > + > + while (prescaler <= 0x80) { > + for (div = 0; div <= 0xF; div++) { > + int x; > + if (prescaler != 0) > + x = (clk_rate / (div + 1)) / (prescaler * 2); > + else > + x = clk_rate / (div + 1); > + > + dev_dbg(&fsl_host->pdev->dev, "x=%d, clock=%d %d\n", > + x, clock, div); > + if (x <= clock) > + break; > + } > + if (div < 0x10) > + break; > + > + prescaler <<= 1; > + } > + dev_dbg(&fsl_host->pdev->dev, "prescaler = 0x%x, divider = 0x%x\n", > + prescaler, div); > + clk |= (prescaler << 8) | (div << 4); > + > + /* Configure the clock control register */ > + clk |= readl(host->ioaddr + SDHCI_CLOCK_CONTROL) > + & (~FSL_SDHCI_CLOCK_MASK); > + clk |= FSL_SDHCI_CLOCK_HLK_EN; > + writel(clk, host->ioaddr + SDHCI_CLOCK_CONTROL); > + > + /* Wait max 10 ms */ > + timeout = 5000; > + while (timeout > 0) { > + timeout--; > + udelay(20); > + } > + writel(clk | FSL_SDHCI_CLOCK_SD_EN, host->ioaddr + > +SDHCI_CLOCK_CONTROL); > + > + host->clock = (clk_rate / (div + 1)) / (prescaler * 2); } Do you know if there is an improvement over the set_clock-routine in sdhic-of-esdhc.c? [<Zhu Richard-r65037>] Accepted. > + > +static void sdhci_fsl_set_power(struct sdhci_host *host, unsigned int > +power) { > + struct sdhci_fsl_host *fsl_host = sdhci_priv(host); > + int voltage = 0; > + > + /* There is no sdhci standard PWR CTL REG in fsl esdhci */ > + if (host->pwr == power) > + return; > + > + if (fsl_host->regulator_mmc) { > + if (power == (unsigned short)-1) { > + regulator_disable(fsl_host->regulator_mmc); > + dev_dbg(&fsl_host->pdev->dev, "mmc power off\n"); > + } else { > + if (power == 7) > + voltage = 1800000; > + else if (power >= 8) > + voltage = 2000000 + (power - 8) * 100000; > + regulator_set_voltage(fsl_host->regulator_mmc, > + voltage, voltage); > + > + if (regulator_enable(fsl_host->regulator_mmc) == 0) { > + dev_dbg(&fsl_host->pdev->dev, "mmc power on\n"); > + msleep(1); > + } > + } > + } > + > + host->pwr = power; > +} The host-structure has a regulator, too. We should use that. Note that my boards don't have such a regulator, so I can't test here. [<Zhu Richard-r65037>] Accepted. > + > +static void sdhci_fsl_set_bus(struct sdhci_host *host, unsigned int > +bus_width) { > + u32 ctrl; > + > + ctrl = readl(host->ioaddr + SDHCI_HOST_CONTROL); > + > + if (bus_width == MMC_BUS_WIDTH_4) { > + ctrl &= ~FSL_SDHCI_CTRL_8BITBUS; > + ctrl |= SDHCI_CTRL_4BITBUS; > + } else if (bus_width == MMC_BUS_WIDTH_8) { > + ctrl &= ~SDHCI_CTRL_4BITBUS; > + ctrl |= FSL_SDHCI_CTRL_8BITBUS; > + } else if (bus_width == MMC_BUS_WIDTH_1) { > + ctrl &= ~SDHCI_CTRL_4BITBUS; > + ctrl &= ~FSL_SDHCI_CTRL_8BITBUS; > + } I'd be careful with the 8Bit-mode for now. We need some to deal with it correctly in sdhci.c first. [<Zhu Richard-r65037>] Are there some problems in the 8bits bus_width in sdhci.c file? I find that the sdhci.c had been support the 8bits bus_width. > + > + writel(ctrl, host->ioaddr + SDHCI_HOST_CONTROL); } > + > +static u16 sdhci_fsl_make_blksz(u16 blk_sz) { > + return blk_sz &= 0x1FFF; > +} > + > +static int esdhci_fsl_enable_dma(struct sdhci_host *host) { > + u32 ctrl; > + > + ctrl = readl(host->ioaddr + SDHCI_HOST_CONTROL); > + ctrl &= ~(SDHCI_CTRL_DMA_MASK << 5); > + if ((host->flags & SDHCI_REQ_USE_DMA) && > + (host->flags & SDHCI_USE_ADMA)) > + ctrl |= (SDHCI_CTRL_ADMA32 << 8); > + else if (host->flags & SDHCI_REQ_USE_DMA) > + ctrl |= (SDHCI_CTRL_SDMA << 8); > + > + writel(ctrl, host->ioaddr + SDHCI_HOST_CONTROL); > + > + if (host->flags & SDHCI_REQ_USE_DMA) > + writel(FSL_SDHCI_WML_64_WORDS, > + host->ioaddr + FSL_SDHCI_WML); > + else > + writel(FSL_SDHCI_WML_128_WORDS, > + host->ioaddr + FSL_SDHCI_WML); > + return 0; > +} > + > +static const struct sdhci_ops sdhci_fsl_ops = { > + .read_l = sdhci_fsl_readl, > + .read_w = sdhci_fsl_readw, > + .read_b = sdhci_fsl_readb, > + .write_l = sdhci_fsl_writel, > + .write_w = sdhci_fsl_writew, > + .write_b = sdhci_fsl_writeb, > + .set_clock = sdhci_fsl_set_clock, > + .set_power = sdhci_fsl_set_power, > + .set_bus = sdhci_fsl_set_bus, > + .enable_dma = esdhci_fsl_enable_dma, > + .get_max_clock = sdhci_fsl_get_max_clock, > + .get_min_clock = sdhci_fsl_get_min_clock, > + .get_timeout_clock = sdhci_fsl_get_timeout_clock, > + .get_ro = sdhci_fsl_get_ro, > + .make_blksz = sdhci_fsl_make_blksz, > +}; > + > +/********************************************************************** *******\ > + * * > + * Device probing/removal * > + * * > +\******************************************************************** > +*********/ > + > +static int __devinit sdhci_fsl_probe(struct platform_device *pdev) { > + struct sdhci_host *host; > + struct resource *iomem; > + struct sdhci_fsl_host *fsl_host; > + int ret; > + struct imxmmc_platform_data *pdata = pdev->dev.platform_data; > + > + BUG_ON(pdev == NULL); > + > + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!iomem) { > + ret = -ENOMEM; > + goto err; > + } > + > + host = sdhci_alloc_host(&pdev->dev, sizeof(struct sdhci_fsl_host)); > + > + if (IS_ERR(host)) { > + ret = PTR_ERR(host); > + goto err; > + } > + > + fsl_host = sdhci_priv(host); > + > + fsl_host->host = host; > + fsl_host->pdev = pdev; > + fsl_host->pdata = pdata; > + > + /* Get clk supply for eSDHC */ > + fsl_host->clk_input = clk_get(&pdev->dev, "esdhc_clk"); > + if (IS_ERR(fsl_host->clk_input)) { > + ret = PTR_ERR(fsl_host->clk_input); > + printk(KERN_ERR "IMX MMC can't get clock.\n"); > + goto err_request_clk; > + } > + clk_enable(fsl_host->clk_input); > + > + dev_dbg(&pdev->dev, "SDHC:%d clock:%lu\n", > + pdev->id, clk_get_rate(fsl_host->clk_input)); > + > + host->hw_name = "sdhci-fsl"; > + host->ops = &sdhci_fsl_ops; > + host->irq = platform_get_irq(pdev, 0); > + > + host->quirks = SDHCI_QUIRK_BROKEN_ADMA Why did you add all the DMA-stuff and mark it as broken then? Is there a problem with the engine? [<Zhu Richard-r65037>] The ADMA is broken, and I still can't find the root cause. I would enable the ADMA feature later. Up to now, the Simple DMA and PIO mode are enabled. > + | SDHCI_QUIRK_32BIT_DMA_ADDR > + | SDHCI_QUIRK_32BIT_ADMA_SIZE > + | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL > + | SDHCI_QUIRK_NO_BUSY_IRQ > + | SDHCI_QUIRK_BROKEN_CARD_DETECTION > + | SDHCI_QUIRK_NONSTANDARD_CLOCK > + | SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET > + | SDHCI_QUIRK_32BIT_CMD_TRANS_COMBINATION > + | SDHCI_QUIRK_NONSTANDARD_HOST_CTL; > + if (!request_mem_region(iomem->start, resource_size(iomem), > + mmc_hostname(host->mmc))) { > + dev_err(&pdev->dev, "cannot request region\n"); > + ret = -EBUSY; > + goto err_request_mem; > + } > + > + host->ioaddr = ioremap(iomem->start, resource_size(iomem)); > + if (!host->ioaddr) { > + dev_err(&pdev->dev, "failed to remap registers\n"); > + ret = -ENOMEM; > + goto err_remap; > + } > + > + ret = sdhci_add_host(host); > + if (ret) > + goto err_add_host; > + > + platform_set_drvdata(pdev, host); > + > + return 0; > + > +err_add_host: > + iounmap(host->ioaddr); > +err_remap: > + release_mem_region(iomem->start, resource_size(iomem)); > +err_request_mem: > + if (fsl_host->clk_input) { > + clk_disable(fsl_host->clk_input); > + clk_put(fsl_host->clk_input); > + } > +err_request_clk: > + if (fsl_host->regulator_mmc) { > + regulator_disable(fsl_host->regulator_mmc); > + regulator_put(fsl_host->regulator_mmc); > + } > + sdhci_free_host(host); > +err: > + printk(KERN_ERR"Probing of sdhci-pltfm failed: %d\n", ret); > + return ret; > +} > + > +static int __devexit sdhci_fsl_remove(struct platform_device *pdev) { > + struct sdhci_host *host = platform_get_drvdata(pdev); > + struct resource *iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + int dead; > + u32 scratch; > + > + dead = 0; > + scratch = readl(host->ioaddr + SDHCI_INT_STATUS); > + if (scratch == (u32)-1) > + dead = 1; > + > + sdhci_remove_host(host, dead); > + iounmap(host->ioaddr); > + release_mem_region(iomem->start, resource_size(iomem)); > + sdhci_free_host(host); > + platform_set_drvdata(pdev, NULL); > + > + return 0; > +} > + > +static struct platform_driver sdhci_fsl_driver = { > + .driver = { > + .name = DRIVER_NAME, > + .owner = THIS_MODULE, > + }, > + .probe = sdhci_fsl_probe, > + .remove = __devexit_p(sdhci_fsl_remove), > +}; > + > +/********************************************************************** *******\ > + * * > + * Driver init/exit * > + * * > +\******************************************************************** > +*********/ > + > +static int __init sdhci_fsl_init(void) { > + return platform_driver_register(&sdhci_fsl_driver); > +} > + > +static void __exit sdhci_fsl_exit(void) { > + platform_driver_unregister(&sdhci_fsl_driver); > +} > + > +module_init(sdhci_fsl_init); > +module_exit(sdhci_fsl_exit); > + > +MODULE_DESCRIPTION("IMX Secure Digital Host Controller Interface > +driver"); MODULE_AUTHOR("Freescale Semiconductor, Inc."); > +MODULE_LICENSE("GPL v2"); MODULE_ALIAS("platform:sdhci-fsl"); > -- > 1.7.0 > > -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html