On Wed, Sep 29, 2010 at 10:27 AM, Zhu Richard-R65037
<[email protected]> wrote:
>
>
>> -----Original Message-----
>> From: zhangfei gao [mailto:[email protected]]
>> Sent: Wednesday, September 29, 2010 10:00 AM
>> To: Ben Dooks
>> Cc: Wolfram Sang; [email protected]; 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 <[email protected]> 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 <[email protected]>
>> 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 <[email protected]>
>> >> > ---
>> >> >
>> >> > 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 <[email protected]>
>> >> > + *
>> >> > + * 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
> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
