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.
>
>> > --- 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
