Re: [PATCH V3 3/3 ] mmc: add support for H/W clock gating of SD controller (resend)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux