On Tue, Dec 7, 2010 at 9:50 PM, Philip Rakity <prakity@xxxxxxxxxxx> wrote: > > On Dec 7, 2010, at 5:49 PM, zhangfei gao wrote: > >> On Tue, Dec 7, 2010 at 7:20 PM, Philip Rakity <prakity@xxxxxxxxxxx> wrote: >>> >>> From 4f521dd851066e6f65cbf3b866502a2a308eb980 Mon Sep 17 00:00:00 2001 >>> From: Philip Rakity <prakity@xxxxxxxxxxx> >>> Date: Tue, 7 Dec 2010 09:05:39 -0800 >>> Subject: [PATCH V2] sdhci: support H/W clock gating in Marvell PXA driver >>> >>> This code is based on sdhci-pxa in mmc-next on Dec 2, 2010. >>> >>> If cpu_is_mmp2() then enable MMC capability MMC_CAP_HW_CLOCK_GATING >>> >>> Code may need changing based on the ongoing work on sdhci-pxa but since that >>> code is NOT in mmc-next better to have something that can work. >>> >>> This code tested on Marvell Linux >>> >>> Signed-off-by: Philip Rakity <prakity@xxxxxxxxxxx> >>> Signed-off-by: Mark F. Brown <markb@xxxxxxxxxxx> >>> --- >>> drivers/mmc/host/sdhci-pxa.c | 33 +++++++++++++++++++++++++++------ >>> 1 files changed, 27 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c >>> index 5a61208..5cf47fd 100644 >>> --- a/drivers/mmc/host/sdhci-pxa.c >>> +++ b/drivers/mmc/host/sdhci-pxa.c >>> @@ -25,6 +25,7 @@ >>> #include <linux/io.h> >>> #include <linux/err.h> >>> #include <plat/sdhci.h> >>> +#include <mach/cputype.h> >>> #include "sdhci.h" >>> >>> #define DRIVER_NAME "sdhci-pxa" >>> @@ -46,10 +47,27 @@ struct sdhci_pxa { >>> * SDHCI core callbacks * >>> * * >>> \*****************************************************************************/ >>> +#ifdef CONFIG_MMC_CLKGATE >>> +static void hardware_clk_gating(struct sdhci_host *host) >>> +{ >>> + unsigned short tmp; >>> + int enable; >>> + >>> + enable = host->mmc->clk_gated; >>> + tmp = readw(host->ioaddr + SD_FIFO_PARAM); >>> + >>> + if (enable) >>> + tmp &= ~DIS_PAD_SD_CLK_GATE; >>> + else >>> + tmp |= DIS_PAD_SD_CLK_GATE; >>> + >>> + writew(tmp, host->ioaddr + SD_FIFO_PARAM); >>> +} >>> +#endif >>> + >>> static void set_clock(struct sdhci_host *host, unsigned int clock) >>> { >>> struct sdhci_pxa *pxa = sdhci_priv(host); >>> - u32 tmp = 0; >>> >>> if (clock == 0) { >>> if (pxa->clk_enable) { >>> @@ -58,11 +76,6 @@ static void set_clock(struct sdhci_host *host, unsigned int clock) >>> } >>> } else { >>> if (0 == pxa->clk_enable) { >>> - if (pxa->pdata->flags & PXA_FLAG_DISABLE_CLOCK_GATING) { >>> - tmp = readl(host->ioaddr + SD_FIFO_PARAM); >>> - tmp |= DIS_PAD_SD_CLK_GATE; >>> - writel(tmp, host->ioaddr + SD_FIFO_PARAM); >>> - } >>> clk_enable(pxa->clk); >>> pxa->clk_enable = 1; >>> } >>> @@ -71,6 +84,9 @@ static void set_clock(struct sdhci_host *host, unsigned int clock) >>> >>> static struct sdhci_ops sdhci_pxa_ops = { >>> .set_clock = set_clock, >>> +#ifdef CONFIG_MMC_CLKGATE >>> + .platform_hw_clk_gate = hardware_clk_gating, >>> +#endif >>> }; >>> >>> /*****************************************************************************\ >>> @@ -145,6 +161,11 @@ static int __devinit sdhci_pxa_probe(struct platform_device *pdev) >>> if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT) >>> host->mmc->caps |= MMC_CAP_8_BIT_DATA; >>> >>> +#ifdef CONFIG_MMC_CLKGATE >>> + if (cpu_is_mmp2()) >>> + host->mmc->caps |= MMC_CAP_HW_CLOCK_GATING; >>> +#endif >>> + >>> ret = sdhci_add_host(host); >>> if (ret) { >>> dev_err(&pdev->dev, "failed to add host\n"); >>> -- >>> 1.6.0.4 >>> >> 1. could you pls follow the interface we discussed before in order to >> prevent the register's difference, .platform_hw_clk_gate could be >> overwrited and transfered from platform, example send before, like >> get_ro. > > We are not in agreement with your proposed implementation. The callbacks in the > driver specific code calling callbacks in the arch code makes the code > difficult to follow. > > It would be much simpler to create 3 entries in kConfig > one for mmp2 > one for pxa910 > one for pxa168 > then factor out the common code. The could then be reused and put it into > a separate file and linked with the SOC specific code to provide the functionality needed. > Rather than the approach of moving > everything into the arch directory. > > It is not clear to us that the approach that you are following can handle the differences > in silicon between the pxa168 and pxa910/mmp2. > > >> 2. we have to fully verify the hardware clk gating internally before >> sending the code. > > code tested under marvell 2.6.32 linux. Thanks a lot, just wander could sdio be supported, since marvell8787 requires around 10 clock cycles after cmd53 finished. > >>> >>> >>> >>> On Dec 7, 2010, at 4:16 PM, Chris Ball wrote: >>> >>>> Hi Philip, >>>> >>>> On Tue, Dec 07, 2010 at 02:46:25PM -0800, Philip Rakity wrote: >>>>> 8 bit width support dependency removed. Code applies directly to mmc-next >>>>> Patches 2/3 and 3/3 unchanged >>>> >>>> The submitted patches 2/3 and 3/3 are identical -- is there a separate >>>> third patch that's unsent so far, or did you mean to send only two? >>>> >>>> Thanks, >>>> >>>> -- >>>> Chris Ball <cjb@xxxxxxxxxx> <http://printf.net/> >>>> One Laptop Per Child >>>> -- >>>> 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 >>> >>> -- >>> 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 >>> > > -- 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