[PATCH] sdhci-s3c: support non-standard clock setting for c210

[Jaehoon Chung]

Kukjin Kim wrote:
> Kyungmin Park wrote:
>   
>> On Thu, Sep 2, 2010 at 7:20 PM, Kukjin Kim <kgene.kim at samsung.com> wrote:
>>     
>>> Jaehoon Chung wrote:
>>>       
>>>> This is sdhci-s3c patch for c210.
>>>> c210 didn't use divider of host controller.
>>>>
>>>> Host Controller need other clock setting methods.
>>>>
>>>> So I add the callback functions for s5pc210.
>>>> also I set 400KHz for initial clock.
>>>>
>>>> Signed-off-by: Jaehoon Chung <jh80.chung at samsung.com>
>>>>  Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
>>>>         
>    ^^^
> Unnecessary whitespace
>
> If your patch includes arch/arm stuff, please add Linux-arm-kernel
> maillinglist.
> (Cc'ed that)
>
>   
i will send to Linux-arm-kernel mailing list.
If you want to test this patch. you also applied following patch.
(ARM: Samsung: Fix header double inclusion)

diff --git a/arch/arm/plat-samsung/include/plat/devs.h b/arch/arm/plat-samsung/include/plat/devs.h
index 85f6f23..42581f8 100644
--- a/arch/arm/plat-samsung/include/plat/devs.h
+++ b/arch/arm/plat-samsung/include/plat/devs.h
@@ -8,7 +8,11 @@
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
-*/
+ */
+
+#ifndef __PLAT_SAMSUNG_DEVS_H
+#define __PLAT_SAMSUNG_DEVS_H
+
 #include <linux/platform_device.h>
 
 struct s3c24xx_uart_resources {
@@ -128,3 +132,5 @@ extern struct platform_device s3c_device_ac97;
  */
 extern void *s3c_set_platdata(void *pd, size_t pdsize,
 			      struct platform_device *pdev);
+
+#endif

And s3c_hsmmc_setname() function could be used in cpu.c file

Thanks
Jaehoon Chung
>>>> ---
>>>>  arch/arm/plat-samsung/include/plat/sdhci.h |   19 ++++++++
>>>>  drivers/mmc/host/sdhci-s3c.c               |   68
>>>> ++++++++++++++++++++++++++++
>>>>  2 files changed, 87 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/arch/arm/plat-samsung/include/plat/sdhci.h
>>>>         
> b/arch/arm/plat-
>   
>>>> samsung/include/plat/sdhci.h
>>>> index 30844c2..7c75ee3 100644
>>>> --- a/arch/arm/plat-samsung/include/plat/sdhci.h
>>>> +++ b/arch/arm/plat-samsung/include/plat/sdhci.h
>>>> @@ -15,6 +15,8 @@
>>>>  #ifndef __PLAT_S3C_SDHCI_H
>>>>  #define __PLAT_S3C_SDHCI_H __FILE__
>>>>
>>>> +#include <plat/devs.h>
>>>> +
>>>>  struct platform_device;
>>>>  struct mmc_host;
>>>>  struct mmc_card;
>>>> @@ -288,4 +290,21 @@ static inline void s5pv210_default_sdhci3(void) {
>>>>         
> }
>   
>>>>  #endif /* CONFIG_S5PV210_SETUP_SDHCI */
>>>>
>>>> +/* re-define device name depending on support. */
>>>> +static inline void s3c_hsmmc_setname(char *name)
>>>> +{
>>>> +#ifdef CONFIG_S3C_DEV_HSMMC
>>>> +     s3c_device_hsmmc0.name = name;
>>>> +#endif
>>>> +#ifdef CONFIG_S3C_DEV_HSMMC1
>>>> +     s3c_device_hsmmc1.name = name;
>>>> +#endif
>>>> +#ifdef CONFIG_S3C_DEV_HSMMC2
>>>> +     s3c_device_hsmmc2.name = name;
>>>> +#endif
>>>> +#ifdef CONFIG_S3C_DEV_HSMMC3
>>>> +     s3c_device_hsmmc3.name = name;
>>>> +#endif
>>>> +}
>>>> +
>>>>  #endif /* __PLAT_S3C_SDHCI_H */
>>>>         
>>> It would be helpful to me if you could separate platform and driver
>>>       
> patches.
>   
i will seperate platform and driver patches, then i will resend to
mailing list.
>>> And if you want to add s3c_hsmmc_setname(), please send together code
>>>       
> that
>   
>>> it is used.
>>>
>>>       
>>>> diff --git a/drivers/mmc/host/sdhci-s3c.c
>>>>         
> b/drivers/mmc/host/sdhci-s3c.c
>   
>>>> index 71ad416..3927793 100644
>>>> --- a/drivers/mmc/host/sdhci-s3c.c
>>>> +++ b/drivers/mmc/host/sdhci-s3c.c
>>>>         
>
> (snip)
>
>   
>>> How do you think about using quirk to separate S5PV310 case as
>>>       
> following?
>   
>>> I think this is more general method in here...And will be submitted soon
>>> after fixing something.
>>>       
>> What's the meaning of "general method"?
>>
>>     
> I'd like to ask you why there is quirk in sdmmc driver.
>
>   
>> and how/where do you set the host->quirks? when board or platform set
>> the these quirks? who know it uses nonstandard quirk?
>>     
>
> How about following?
>
> static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
> ...
> +	if (pdev->id_entry->driver_data == TYPE_S5PV310)
> +		host->quirks |= SDHCI_QUIRK_BROKEN_CLOCK_DIVIDER;
> ...
>
>   
>> In case s5pc210 don't have standard host controller. it's more clear
>> to use own functions instead of quirks.
>>
>>     
> Why do we add another callback function?
>
>   
>>> From: Hyuk Lee <hyuk1.lee at samsung.com>
>>>
>>> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
>>> index 71ad416..1ac2f36 100644
>>> --- a/drivers/mmc/host/sdhci-s3c.c
>>> +++ b/drivers/mmc/host/sdhci-s3c.c
>>>       
>
> (snip)
>
>   
>>> @@ -221,6 +257,11 @@ static unsigned int sdhci_s3c_get_min_clock(struct
>>> sdhci_host *host)
>>>        unsigned int delta, min = UINT_MAX;
>>>        int src;
>>>
>>> +       /* There is only one clock source(sclk) if there is no clock
>>>       
> divider
>   
>>> +        * in the host controller */
>>> +       if(host->quirks & SDHCI_QUIRK_BROKEN_CLOCK_DIVIDER)
>>> +               return clk_round_rate(ourhost->clk_bus[2], 400000);
>>>       
>> what's the clk_bus[2]?
>>
>>     
> Should be clk_bus[ourhost->cur_clk]
>
> (snip)
>
> Best regards,
> Kgene.
> --
> Kukjin Kim <kgene.kim at samsung.com>, Senior Engineer,
> SW Solution Development Team, Samsung Electronics Co., Ltd.
>
>
>