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

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

 



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


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

  Powered by Linux