On Dec 7, 2010, at 11:13 PM, zhangfei gao wrote:
> 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.
my understanding is that the 8787 requires 12 clocks not 10. The spec calls for 8 clocks.
Hardware clock gating sdio does not work with 8787. We have tested this and know it fails.
>
>>
>>>>
>>>>
>>>>
>>>> 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
