Kyungmin Park wrote:
>
> On Thu, Sep 2, 2010 at 7:20 PM, Kukjin Kim <kgene.kim@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> >> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
^^^
Unnecessary whitespace
If your patch includes arch/arm stuff, please add Linux-arm-kernel
maillinglist.
(Cc'ed that)
> >>
> >> ---
> >> 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.
> > 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@xxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxx>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
--
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
