> -----Original Message----- > From: zhangfei gao [mailto:zhangfei.gao@xxxxxxxxx] > Sent: Wednesday, September 29, 2010 11:22 AM > To: Zhu Richard-R65037 > Cc: Ben Dooks; Wolfram Sang; linux-mmc@xxxxxxxxxxxxxxx; Anton Vorontsov; > Philip Rakity; Eric Miao; Haojian Zhuang; Saeed Bishara > Subject: Re: [PATCH 6/6] mmc: sdhci-pltfm: add pltfm-driver for imx35/51 > > On Wed, Sep 29, 2010 at 10:27 AM, Zhu Richard-R65037 > <r65037@xxxxxxxxxxxxx> wrote: > > > > > >> -----Original Message----- > >> From: zhangfei gao [mailto:zhangfei.gao@xxxxxxxxx] > >> Sent: Wednesday, September 29, 2010 10:00 AM > >> To: Ben Dooks > >> Cc: Wolfram Sang; linux-mmc@xxxxxxxxxxxxxxx; Anton Vorontsov; Zhu > >> Richard-R65037; Philip Rakity; Eric Miao; Haojian Zhuang; Saeed > >> Bishara > >> Subject: Re: [PATCH 6/6] mmc: sdhci-pltfm: add pltfm-driver for > >> imx35/51 > >> > >> On Wed, Sep 29, 2010 at 4:57 AM, Ben Dooks <ben@xxxxxxxxxxxxxxxxx> > wrote: > >> > On Tue, Sep 28, 2010 at 10:04:14AM -0400, zhangfei gao wrote: > >> >> On Tue, Sep 28, 2010 at 8:36 AM, Wolfram Sang > >> >> <w.sang@xxxxxxxxxxxxxx> > >> wrote: > >> >> > This driver adds basic support for the esdhc-core found on e.g. > >> >> > imx35/51. It adds up to the pltfm-core. > >> >> > > >> >> > Signed-off-by: Wolfram Sang <w.sang@xxxxxxxxxxxxxx> > >> >> > --- > >> >> > > >> >> > Changes since last version: > >> >> > > >> >> > * some "-imx" suffixes added > >> >> > * some cosmetic improvements > >> >> > > >> >> > Thanks to Anton for those. > >> >> > > >> >> > drivers/mmc/host/Kconfig | 10 +++ > >> >> > drivers/mmc/host/Makefile | 1 + > >> >> > drivers/mmc/host/sdhci-esdhc-imx.c | 144 > >> >> > ++++++++++++++++++++++++++++++++++++ > >> >> > drivers/mmc/host/sdhci-pltfm.c | 3 + > >> >> > drivers/mmc/host/sdhci-pltfm.h | 1 + > >> >> > 5 files changed, 159 insertions(+), 0 deletions(-) > >> >> > create mode 100644 drivers/mmc/host/sdhci-esdhc-imx.c > >> >> > > >> >> > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig > >> >> > index 6f12d5d..815bf0f 100644 > >> >> > --- a/drivers/mmc/host/Kconfig > >> >> > +++ b/drivers/mmc/host/Kconfig > >> >> > @@ -143,6 +143,16 @@ config MMC_SDHCI_MV > >> >> > > >> >> > If unsure, say N. > >> >> > > >> >> > +config MMC_SDHCI_ESDHC > >> >> > + bool "SDHCI platform support for the Freescale eSDHC > >> controller" > >> >> > + depends on MMC_SDHCI_PLTFM > >> >> > + select MMC_SDHCI_IO_ACCESSORS > >> >> > + help > >> >> > + This selects the Freescale eSDHC controller support on > >> >> > +the platform > >> >> > + bus, found on platforms like mx35/51. > >> >> > + > >> >> > + If unsure, say N. > >> >> > + > >> >> > config MMC_SDHCI_S3C > >> >> > tristate "SDHCI support on Samsung S3C SoC" > >> >> > depends on MMC_SDHCI && PLAT_SAMSUNG diff --git > >> >> > a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile index > >> >> > ef32c32..5294425 100644 > >> >> > --- a/drivers/mmc/host/Makefile > >> >> > +++ b/drivers/mmc/host/Makefile > >> >> > @@ -38,6 +38,7 @@ obj-$(CONFIG_MMC_USHC) += > >> >> > ushc.o > >> >> > obj-$(CONFIG_MMC_SDHCI_PLTFM) += > >> >> > sdhci-platform.o > >> >> > sdhci-platform-y := sdhci-pltfm.o > >> >> > sdhci-platform-$(CONFIG_MMC_SDHCI_CNS3XXX) += > >> >> > sdhci-cns3xxx.o > >> >> > +sdhci-platform-$(CONFIG_MMC_SDHCI_ESDHC) += > >> >> > +sdhci-esdhc-imx.o > >> >> > > >> >> > obj-$(CONFIG_MMC_SDHCI_OF) += sdhci-of.o > >> >> > sdhci-of-y := sdhci-of-core.o diff > >> >> > --git a/drivers/mmc/host/sdhci-esdhc-imx.c > >> >> > b/drivers/mmc/host/sdhci-esdhc-imx.c > >> >> > new file mode 100644 > >> >> > index 0000000..c43d448 > >> >> > --- /dev/null > >> >> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > >> >> > @@ -0,0 +1,144 @@ > >> >> > +/* > >> >> > + * Freescale eSDHC controller driver for the platform bus. > >> >> > + * > >> >> > + * derived from the OF-version. > >> >> > + * > >> >> > + * Copyright (c) 2010 Pengutronix e.K. > >> >> > + * Author: Wolfram Sang <w.sang@xxxxxxxxxxxxxx> > >> >> > + * > >> >> > + * 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. > >> >> > + */ > >> >> > + > >> >> > +#include <linux/io.h> > >> >> > +#include <linux/delay.h> > >> >> > +#include <linux/err.h> > >> >> > +#include <linux/clk.h> > >> >> > +#include <linux/mmc/host.h> > >> >> > +#include <linux/mmc/sdhci-pltfm.h> #include "sdhci.h" > >> >> > +#include "sdhci-pltfm.h" > >> >> > +#include "sdhci-esdhc.h" > >> >> > + > >> >> > +static inline void esdhc_clrset_le(struct sdhci_host *host, u32 > >> >> > +mask, u32 val, int reg) { > >> >> > + void __iomem *base = host->ioaddr + (reg & ~0x3); > >> >> > + u32 shift = (reg & 0x3) * 8; > >> >> > + > >> >> > + writel(((readl(base) & ~(mask << shift)) | (val << > >> >> > +shift)), base); } > >> >> > + > >> >> > +static u16 esdhc_readw_le(struct sdhci_host *host, int reg) { > >> >> > + if (unlikely(reg == SDHCI_HOST_VERSION)) > >> >> > + reg ^= 2; > >> >> > + > >> >> > + return readw(host->ioaddr + reg); } > >> >> > + > >> >> > +static void esdhc_writew_le(struct sdhci_host *host, u16 val, > >> >> > +int > >> >> > +reg) { > >> >> > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > >> >> > + > >> >> > + switch (reg) { > >> >> > + case SDHCI_TRANSFER_MODE: > >> >> > + /* > >> >> > + * Postpone this write, we must do it together > >> >> > +with a > >> >> > + * command write that is down below. > >> >> > + */ > >> >> > + pltfm_host->scratchpad = val; > >> >> > + return; > >> >> > + case SDHCI_COMMAND: > >> >> > + writel(val << 16 | pltfm_host->scratchpad, > >> >> > + host->ioaddr + SDHCI_TRANSFER_MODE); > >> >> > + return; > >> >> > + case SDHCI_BLOCK_SIZE: > >> >> > + val &= ~SDHCI_MAKE_BLKSZ(0x7, 0); > >> >> > + break; > >> >> > + } > >> >> > + esdhc_clrset_le(host, 0xffff, val, reg); } > >> >> > + > >> >> > +static void esdhc_writeb_le(struct sdhci_host *host, u8 val, > >> >> > +int > >> >> > +reg) { > >> >> > + u32 new_val; > >> >> > + > >> >> > + switch (reg) { > >> >> > + case SDHCI_POWER_CONTROL: > >> >> > + /* > >> >> > + * FSL put some DMA bits here > >> >> > + * If your board has a regulator, code should be > >> >> > + here > >> >> > + */ > >> >> > + return; > >> >> > + case SDHCI_HOST_CONTROL: > >> >> > + /* FSL messed up here, so we can just keep those > >> >> > + two */ > >> >> > + new_val = val & (SDHCI_CTRL_LED | > >> >> > + SDHCI_CTRL_4BITBUS); > >> >> > + /* ensure the endianess */ > >> >> > + new_val |= ESDHC_HOST_CONTROL_LE; > >> >> > + /* DMA mode bits are shifted */ > >> >> > + new_val |= (val & SDHCI_CTRL_DMA_MASK) << 5; > >> >> > + > >> >> > + esdhc_clrset_le(host, 0xffff, new_val, reg); > >> >> > + return; > >> >> > + } > >> >> > + esdhc_clrset_le(host, 0xff, val, reg); } > >> >> > + > >> >> > +static unsigned int esdhc_pltfm_get_max_clock(struct sdhci_host > >> >> > +*host) { > >> >> > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > >> >> > + > >> >> > + return clk_get_rate(pltfm_host->clk); } > >> >> > + > >> >> > +static unsigned int esdhc_pltfm_get_min_clock(struct sdhci_host > >> >> > +*host) { > >> >> > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > >> >> > + > >> >> > + return clk_get_rate(pltfm_host->clk) / 256 / 16; } > >> > > >> > the above always confuses me, how about a / (b * c) or similar? > >> > > >> >> > +static int esdhc_pltfm_init(struct sdhci_host *host, > >> >> > + struct sdhci_pltfm_data *pdata, void > >> >> > +*priv_pdata) { > >> >> > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > >> >> > + struct clk *clk; > >> >> > + > >> >> > + clk = clk_get(NULL, "sdhc"); > >> >> > >> >> Here only could support one device, how about several device, for > >> >> example sdhc.0, sdhc.1 sdhc.2, if using this method, they will get > >> >> the first clock. > >> >> Then sdhc.1 sdhc.2 can not work. > >> > > >> > No, clk_get() should be passed the device and a NULL for preference. > >> > > >> > The main match should be made on the device, and the name is an > >> > optional distinguisher for devices that have multiple clock sources. > >> > >> Sorry, not understand, > >> If three devices, sdhci,0, sdhci.1, sdhci.2, each device have > >> different clock, with different register. > >> Pass NULL will not distinguish which device, so can not choose which > clk. > >> > > Hi: > > How about to add such kind of device_find_child func codes that can > > indicated > > which dev is requesting the clk. > > static int __match_sdhci_imx(struct device *dev, void *data) { > > return !strncmp(dev_name(dev), "imx-sdhci", 9); } > > They are same effect, what you get is only the first one, imx-sdhci.0 The clk name string can be set and transferred from the private board specified platform data. We can just use the device_find_child func to find the different dev by dev_id. (BTW, the dev.parent would be used when allocate the host.) These example codes are used when I verify the functions on the i.MX51 BBG board based on the platform Core sdhci-pltfm.c file. I found that device can't get the proper clk when the first parameter of the clk_get func is NULL. So I add these codes in my local branch to verify the basic functions on the i.MX51 board. One more complementary, the different clocks should have the unified interfaces although they have the different Registers or the configurations. > > > static int sdhci_imx_init(struct sdhci_host *host) { > > struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > > struct clk *clk; > > struct device *dev; > > > > dev = device_find_child(mmc_dev(host->mmc), NULL, > > __match_sdhci_imx); > > clk = clk_get(dev, "imx_sdhc_clk"); > > if (IS_ERR(clk)) { > > dev_err(mmc_dev(host->mmc), "clk err\n"); > > return -ENODEV; > > } > > clk_enable(clk); > > pltfm_host->clk = clk; > > dev_dbg(dev, "SDHC clock:%lu\n", clk_get_rate(clk)); > > > > return 0; > > } > >> > > >> >> > --- a/drivers/mmc/host/sdhci-pltfm.c > >> >> > +++ b/drivers/mmc/host/sdhci-pltfm.c > >> >> > @@ -169,6 +169,9 @@ static const struct platform_device_id > >> >> > sdhci_pltfm_ids[] = { > >> >> > #ifdef CONFIG_MMC_SDHCI_CNS3XXX > >> >> > { "sdhci-cns3xxx", (kernel_ulong_t)&sdhci_cns3xxx_pdata > >> >> > }, > >> >> > #endif > >> >> > +#ifdef CONFIG_MMC_SDHCI_ESDHC > >> >> > + { "sdhci-esdhc-imx", > >> >> > +(kernel_ulong_t)&sdhci_esdhc_imx_pdata > >> >> > +}, #endif > >> >> > { }, > >> > > >> > I'd much prefer to see these outside of the sdhci-pltfm.c, this > >> > sort of #ifdef went out of other drivers with the end of the caveman. > >> > > >> > My preference would be to rework the sdhci-pltfm.c to be the 'core' > >> > implementation called by each driver. I'll have a look at posting > >> > some patches to see what can be done to sort this out. > >> > > >> > -- > >> > Ben > >> > > >> > Q: What's a light-year? > >> > A: One-third less calories than a regular year. > >> > > >> > > > > > > > -- 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
