[PATCH v2 0/2] sdhci-s3c: Add support S5PV310/S5PC210 HSMMC

[PATCH v2 0/2] sdhci-s3c: Add support S5PV310/S5PC210 HSMMC

[Kukjin Kim]

This patch adds support S5PV310/S5PC210 HSMMC and additional host capabilities.

HSMMC of S5PV310 does not have internal clock divider so need to update for it.
(used SDHCI_QUIRK_NONSTANDARD_CLOCK quirk and added clk_type plat data)

Changes since v1:
- Separated to each topic

[PATCH v2 1/2] sdhci-s3c: Add support additional host capabilities
[PATCH v2 2/2] sdhci-s3c: Add support no internal clock divider in host controller

[PATCH v2 0/2] sdhci-s3c: Add support S5PV310/S5PC210 HSMMC

[Kukjin Kim]

This patch adds support S5PV310/S5PC210 HSMMC and additional host capabilities.

HSMMC of S5PV310 does not have internal clock divider so need to update for it.
(used SDHCI_QUIRK_NONSTANDARD_CLOCK quirk and added clk_type plat data)

Changes since v1:
- Separated to each topic

[PATCH v2 1/2] sdhci-s3c: Add support additional host capabilities
[PATCH v2 2/2] sdhci-s3c: Add support no internal clock divider in host controller

[PATCH v2 1/2] sdhci-s3c: Add support additional host capabilities

[Kukjin Kim]

From: Jeongbae Seo <jeongbae.seo@samsung.com>

This patch adds to support additional host capabilities
like SD/MMC high speed, SDHCI bus width and etc.

Signed-off-by: Jeongbae Seo <jeongbae.seo@samsung.com>
Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
---
 drivers/mmc/host/sdhci-s3c.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 71ad416..e6e0438 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -425,6 +425,10 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	/* HSMMC on Samsung SoCs uses SDCLK as timeout clock */
 	host->quirks |= SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK;
 
+	/* It supports additional host capabilities if needed */
+	if (pdata->host_caps)
+		host->mmc->caps |= pdata->host_caps;
+
 	ret = sdhci_add_host(host);
 	if (ret) {
 		dev_err(dev, "sdhci_add_host() failed\n");
-- 
1.6.2.5

[PATCH v2 1/2] sdhci-s3c: Add support additional host capabilities

[Kukjin Kim]

From: Jeongbae Seo <jeongbae.seo@samsung.com>

This patch adds to support additional host capabilities
like SD/MMC high speed, SDHCI bus width and etc.

Signed-off-by: Jeongbae Seo <jeongbae.seo@samsung.com>
Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
---
 drivers/mmc/host/sdhci-s3c.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 71ad416..e6e0438 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -425,6 +425,10 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	/* HSMMC on Samsung SoCs uses SDCLK as timeout clock */
 	host->quirks |= SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK;
 
+	/* It supports additional host capabilities if needed */
+	if (pdata->host_caps)
+		host->mmc->caps |= pdata->host_caps;
+
 	ret = sdhci_add_host(host);
 	if (ret) {
 		dev_err(dev, "sdhci_add_host() failed\n");
-- 
1.6.2.5

[PATCH v2 2/2] sdhci-s3c: Add support no internal clock divider in host controller

[Kukjin Kim]

From: Hyuk Lee <hyuk1.lee@samsung.com>

This patch adds to support no internal clock divider in SDHCI.
The external clock divider can be used to make a proper clock
because SDHCI doesn't support internal clock divider by itself.

Signed-off-by: Hyuk Lee <hyuk1.lee@samsung.com>
Signed-off-by: Jeongbae Seo <jeongbae.seo@samsung.com>
Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
---
 drivers/mmc/host/sdhci-s3c.c |   60 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index e6e0438..5ad5ed7 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -96,6 +96,13 @@ static unsigned int sdhci_s3c_get_max_clk(struct sdhci_host *host)
 	unsigned int rate, max;
 	int clk;
 
+	/*
+	 * There is only one clock source(sclk) if there is no clock divider
+	 * in the host controller
+	 */
+	if (host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK)
+		return clk_round_rate(ourhost->clk_bus[2], UINT_MAX);
+
 	/* note, a reset will reset the clock source */
 
 	sdhci_s3c_check_sclk(host);
@@ -130,6 +137,15 @@ static unsigned int sdhci_s3c_consider_clock(struct sdhci_s3c *ourhost,
 	if (!clksrc)
 		return UINT_MAX;
 
+	/*
+	 * There is only one clock source(sclk) if there is no clock divider
+	 * in the host controller
+	 */
+	if (ourhost->host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK) {
+		rate = clk_round_rate(clksrc, wanted);
+		return wanted - rate;
+	}
+
 	rate = clk_get_rate(clksrc);
 
 	for (div = 1; div < 256; div *= 2) {
@@ -159,6 +175,7 @@ static void sdhci_s3c_set_clock(struct sdhci_host *host, unsigned int clock)
 	int best_src = 0;
 	int src;
 	u32 ctrl;
+	unsigned int timeout;
 
 	/* don't bother if the clock is going off. */
 	if (clock == 0)
@@ -204,6 +221,35 @@ static void sdhci_s3c_set_clock(struct sdhci_host *host, unsigned int clock)
 			(ourhost->pdata->cfg_card)(ourhost->pdev, host->ioaddr,
 						   &ios, NULL);
 	}
+
+	/*
+	 * There is only one clock source(sclk) if there is no clock divider
+	 * in the host controller
+	 */
+	if (host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK) {
+		writew(0, host->ioaddr + SDHCI_CLOCK_CONTROL);
+		clk_set_rate(ourhost->clk_bus[ourhost->cur_clk], clock);
+
+		writew(SDHCI_CLOCK_INT_EN, host->ioaddr + SDHCI_CLOCK_CONTROL);
+
+		/* Wait max 20 ms */
+		timeout = 20;
+		while (!((sdhci_readw(host, SDHCI_CLOCK_CONTROL))
+			& SDHCI_CLOCK_INT_STABLE)) {
+			if (timeout == 0) {
+				printk(KERN_ERR "%s: clock never stabilised.\n"
+						, mmc_hostname(host->mmc));
+				return;
+			}
+			timeout--;
+			mdelay(1);
+		}
+
+		writew(SDHCI_CLOCK_INT_EN | SDHCI_CLOCK_CARD_EN,
+				host->ioaddr + SDHCI_CLOCK_CONTROL);
+
+		host->clock = clock;
+	}
 }
 
 /**
@@ -221,6 +267,13 @@ 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_NONSTANDARD_CLOCK)
+		return clk_round_rate(ourhost->clk_bus[2], 400000);
+
 	for (src = 0; src < MAX_BUS_CLK; src++) {
 		delta = sdhci_s3c_consider_clock(ourhost, src, 0);
 		if (delta == UINT_MAX)
@@ -425,6 +478,13 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	/* HSMMC on Samsung SoCs uses SDCLK as timeout clock */
 	host->quirks |= SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK;
 
+	/*
+	 * If controller does not have internal clock divider,
+	 * we need to use another method with setup a quirk.
+	 */
+	if (pdata->clk_type)
+		host->quirks |= SDHCI_QUIRK_NONSTANDARD_CLOCK;
+
 	/* It supports additional host capabilities if needed */
 	if (pdata->host_caps)
 		host->mmc->caps |= pdata->host_caps;
-- 
1.6.2.5

[PATCH v2 2/2] sdhci-s3c: Add support no internal clock divider in host controller

[Kukjin Kim]

From: Hyuk Lee <hyuk1.lee@samsung.com>

This patch adds to support no internal clock divider in SDHCI.
The external clock divider can be used to make a proper clock
because SDHCI doesn't support internal clock divider by itself.

Signed-off-by: Hyuk Lee <hyuk1.lee@samsung.com>
Signed-off-by: Jeongbae Seo <jeongbae.seo@samsung.com>
Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
---
 drivers/mmc/host/sdhci-s3c.c |   60 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index e6e0438..5ad5ed7 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -96,6 +96,13 @@ static unsigned int sdhci_s3c_get_max_clk(struct sdhci_host *host)
 	unsigned int rate, max;
 	int clk;
 
+	/*
+	 * There is only one clock source(sclk) if there is no clock divider
+	 * in the host controller
+	 */
+	if (host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK)
+		return clk_round_rate(ourhost->clk_bus[2], UINT_MAX);
+
 	/* note, a reset will reset the clock source */
 
 	sdhci_s3c_check_sclk(host);
@@ -130,6 +137,15 @@ static unsigned int sdhci_s3c_consider_clock(struct sdhci_s3c *ourhost,
 	if (!clksrc)
 		return UINT_MAX;
 
+	/*
+	 * There is only one clock source(sclk) if there is no clock divider
+	 * in the host controller
+	 */
+	if (ourhost->host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK) {
+		rate = clk_round_rate(clksrc, wanted);
+		return wanted - rate;
+	}
+
 	rate = clk_get_rate(clksrc);
 
 	for (div = 1; div < 256; div *= 2) {
@@ -159,6 +175,7 @@ static void sdhci_s3c_set_clock(struct sdhci_host *host, unsigned int clock)
 	int best_src = 0;
 	int src;
 	u32 ctrl;
+	unsigned int timeout;
 
 	/* don't bother if the clock is going off. */
 	if (clock == 0)
@@ -204,6 +221,35 @@ static void sdhci_s3c_set_clock(struct sdhci_host *host, unsigned int clock)
 			(ourhost->pdata->cfg_card)(ourhost->pdev, host->ioaddr,
 						   &ios, NULL);
 	}
+
+	/*
+	 * There is only one clock source(sclk) if there is no clock divider
+	 * in the host controller
+	 */
+	if (host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK) {
+		writew(0, host->ioaddr + SDHCI_CLOCK_CONTROL);
+		clk_set_rate(ourhost->clk_bus[ourhost->cur_clk], clock);
+
+		writew(SDHCI_CLOCK_INT_EN, host->ioaddr + SDHCI_CLOCK_CONTROL);
+
+		/* Wait max 20 ms */
+		timeout = 20;
+		while (!((sdhci_readw(host, SDHCI_CLOCK_CONTROL))
+			& SDHCI_CLOCK_INT_STABLE)) {
+			if (timeout == 0) {
+				printk(KERN_ERR "%s: clock never stabilised.\n"
+						, mmc_hostname(host->mmc));
+				return;
+			}
+			timeout--;
+			mdelay(1);
+		}
+
+		writew(SDHCI_CLOCK_INT_EN | SDHCI_CLOCK_CARD_EN,
+				host->ioaddr + SDHCI_CLOCK_CONTROL);
+
+		host->clock = clock;
+	}
 }
 
 /**
@@ -221,6 +267,13 @@ 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_NONSTANDARD_CLOCK)
+		return clk_round_rate(ourhost->clk_bus[2], 400000);
+
 	for (src = 0; src < MAX_BUS_CLK; src++) {
 		delta = sdhci_s3c_consider_clock(ourhost, src, 0);
 		if (delta == UINT_MAX)
@@ -425,6 +478,13 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	/* HSMMC on Samsung SoCs uses SDCLK as timeout clock */
 	host->quirks |= SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK;
 
+	/*
+	 * If controller does not have internal clock divider,
+	 * we need to use another method with setup a quirk.
+	 */
+	if (pdata->clk_type)
+		host->quirks |= SDHCI_QUIRK_NONSTANDARD_CLOCK;
+
 	/* It supports additional host capabilities if needed */
 	if (pdata->host_caps)
 		host->mmc->caps |= pdata->host_caps;
-- 
1.6.2.5

Re: [PATCH v2 2/2] sdhci-s3c: Add support no internal clock divider in host controller

[Ben Dooks]

On 17/09/10 10:45, Kukjin Kim wrote:
> From: Hyuk Lee <hyuk1.lee@samsung.com>
> 
> This patch adds to support no internal clock divider in SDHCI.
> The external clock divider can be used to make a proper clock
> because SDHCI doesn't support internal clock divider by itself.
> 
> Signed-off-by: Hyuk Lee <hyuk1.lee@samsung.com>
> Signed-off-by: Jeongbae Seo <jeongbae.seo@samsung.com>
> Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> ---
>  drivers/mmc/host/sdhci-s3c.c |   60 ++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 60 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index e6e0438..5ad5ed7 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -96,6 +96,13 @@ static unsigned int sdhci_s3c_get_max_clk(struct sdhci_host *host)
>  	unsigned int rate, max;
>  	int clk;
>  
> +	/*
> +	 * There is only one clock source(sclk) if there is no clock divider
> +	 * in the host controller
> +	 */
> +	if (host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK)
> +		return clk_round_rate(ourhost->clk_bus[2], UINT_MAX);

interesting, <linux/clk.h> doesn't have a second parameter to
clk_round_rate().


>  	/* note, a reset will reset the clock source */
>  
>  	sdhci_s3c_check_sclk(host);
> @@ -130,6 +137,15 @@ static unsigned int sdhci_s3c_consider_clock(struct sdhci_s3c *ourhost,
>  	if (!clksrc)
>  		return UINT_MAX;
>  
> +	/*
> +	 * There is only one clock source(sclk) if there is no clock divider
> +	 * in the host controller
> +	 */
> +	if (ourhost->host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK) {
> +		rate = clk_round_rate(clksrc, wanted);
> +		return wanted - rate;
> +	}

Why does this need a quirk, instead of just having one clock available
in the list of usable clocks?

-- 
Ben

[PATCH v2 2/2] sdhci-s3c: Add support no internal clock divider in host controller

[Ben Dooks]

On 17/09/10 10:45, Kukjin Kim wrote:
> From: Hyuk Lee <hyuk1.lee@samsung.com>
> 
> This patch adds to support no internal clock divider in SDHCI.
> The external clock divider can be used to make a proper clock
> because SDHCI doesn't support internal clock divider by itself.
> 
> Signed-off-by: Hyuk Lee <hyuk1.lee@samsung.com>
> Signed-off-by: Jeongbae Seo <jeongbae.seo@samsung.com>
> Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> ---
>  drivers/mmc/host/sdhci-s3c.c |   60 ++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 60 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index e6e0438..5ad5ed7 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -96,6 +96,13 @@ static unsigned int sdhci_s3c_get_max_clk(struct sdhci_host *host)
>  	unsigned int rate, max;
>  	int clk;
>  
> +	/*
> +	 * There is only one clock source(sclk) if there is no clock divider
> +	 * in the host controller
> +	 */
> +	if (host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK)
> +		return clk_round_rate(ourhost->clk_bus[2], UINT_MAX);

interesting, <linux/clk.h> doesn't have a second parameter to
clk_round_rate().


>  	/* note, a reset will reset the clock source */
>  
>  	sdhci_s3c_check_sclk(host);
> @@ -130,6 +137,15 @@ static unsigned int sdhci_s3c_consider_clock(struct sdhci_s3c *ourhost,
>  	if (!clksrc)
>  		return UINT_MAX;
>  
> +	/*
> +	 * There is only one clock source(sclk) if there is no clock divider
> +	 * in the host controller
> +	 */
> +	if (ourhost->host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK) {
> +		rate = clk_round_rate(clksrc, wanted);
> +		return wanted - rate;
> +	}

Why does this need a quirk, instead of just having one clock available
in the list of usable clocks?

-- 
Ben

RE: [PATCH v2 2/2] sdhci-s3c: Add support no internal clock divider in host controller

[Jeongbae Seo]

Ben Dooks worte:
> On 17/09/10 10:45, Kukjin Kim wrote:
> > From: Hyuk Lee <hyuk1.lee@samsung.com>
> >
> > This patch adds to support no internal clock divider in SDHCI.
> > The external clock divider can be used to make a proper clock
> > because SDHCI doesn't support internal clock divider by itself.
> >
> > Signed-off-by: Hyuk Lee <hyuk1.lee@samsung.com>
> > Signed-off-by: Jeongbae Seo <jeongbae.seo@samsung.com>
> > Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> > ---
> >  drivers/mmc/host/sdhci-s3c.c |   60
> ++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 60 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> > index e6e0438..5ad5ed7 100644
> > --- a/drivers/mmc/host/sdhci-s3c.c
> > +++ b/drivers/mmc/host/sdhci-s3c.c
> > @@ -96,6 +96,13 @@ static unsigned int sdhci_s3c_get_max_clk(struct
> sdhci_host *host)
> >  	unsigned int rate, max;
> >  	int clk;
> >
> > +	/*
> > +	 * There is only one clock source(sclk) if there is no clock
> divider
> > +	 * in the host controller
> > +	 */
> > +	if (host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK)
> > +		return clk_round_rate(ourhost->clk_bus[2], UINT_MAX);
> 
> interesting, <linux/clk.h> doesn't have a second parameter to
> clk_round_rate().
> 
Hi Ben,

Thanks for your comments.

When I see clk_round_rate in <linux/clk.h>, which function is defined 
as "long clk_round_rate(struct clk *clk, unsigned long rate)" that has two
parameters.
Please let me know if you have another meaning for this.

> 
> >  	/* note, a reset will reset the clock source */
> >
> >  	sdhci_s3c_check_sclk(host);
> > @@ -130,6 +137,15 @@ static unsigned int sdhci_s3c_consider_clock(struct
> sdhci_s3c *ourhost,
> >  	if (!clksrc)
> >  		return UINT_MAX;
> >
> > +	/*
> > +	 * There is only one clock source(sclk) if there is no clock
> divider
> > +	 * in the host controller
> > +	 */
> > +	if (ourhost->host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK) {
> > +		rate = clk_round_rate(clksrc, wanted);
> > +		return wanted - rate;
> > +	}
> 
> Why does this need a quirk, instead of just having one clock available
> in the list of usable clocks?
> 

The available clock is made by dividing a clock source with a certain
divider value.
Most of the host controller has this capability that can divide a clock what
we want.
However, the host controller of both S5PC210 and S5PV310 don't have this so
we have to
Add additional routine to make a proper clock with outland clock divider
instead of
Internal clock divider in host controller.

Best Regards,
Jeongbae Seo

[PATCH v2 2/2] sdhci-s3c: Add support no internal clock divider in host controller

[Jeongbae Seo]

Ben Dooks worte:
> On 17/09/10 10:45, Kukjin Kim wrote:
> > From: Hyuk Lee <hyuk1.lee@samsung.com>
> >
> > This patch adds to support no internal clock divider in SDHCI.
> > The external clock divider can be used to make a proper clock
> > because SDHCI doesn't support internal clock divider by itself.
> >
> > Signed-off-by: Hyuk Lee <hyuk1.lee@samsung.com>
> > Signed-off-by: Jeongbae Seo <jeongbae.seo@samsung.com>
> > Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> > ---
> >  drivers/mmc/host/sdhci-s3c.c |   60
> ++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 60 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> > index e6e0438..5ad5ed7 100644
> > --- a/drivers/mmc/host/sdhci-s3c.c
> > +++ b/drivers/mmc/host/sdhci-s3c.c
> > @@ -96,6 +96,13 @@ static unsigned int sdhci_s3c_get_max_clk(struct
> sdhci_host *host)
> >  	unsigned int rate, max;
> >  	int clk;
> >
> > +	/*
> > +	 * There is only one clock source(sclk) if there is no clock
> divider
> > +	 * in the host controller
> > +	 */
> > +	if (host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK)
> > +		return clk_round_rate(ourhost->clk_bus[2], UINT_MAX);
> 
> interesting, <linux/clk.h> doesn't have a second parameter to
> clk_round_rate().
> 
Hi Ben,

Thanks for your comments.

When I see clk_round_rate in <linux/clk.h>, which function is defined 
as "long clk_round_rate(struct clk *clk, unsigned long rate)" that has two
parameters.
Please let me know if you have another meaning for this.

> 
> >  	/* note, a reset will reset the clock source */
> >
> >  	sdhci_s3c_check_sclk(host);
> > @@ -130,6 +137,15 @@ static unsigned int sdhci_s3c_consider_clock(struct
> sdhci_s3c *ourhost,
> >  	if (!clksrc)
> >  		return UINT_MAX;
> >
> > +	/*
> > +	 * There is only one clock source(sclk) if there is no clock
> divider
> > +	 * in the host controller
> > +	 */
> > +	if (ourhost->host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK) {
> > +		rate = clk_round_rate(clksrc, wanted);
> > +		return wanted - rate;
> > +	}
> 
> Why does this need a quirk, instead of just having one clock available
> in the list of usable clocks?
> 

The available clock is made by dividing a clock source with a certain
divider value.
Most of the host controller has this capability that can divide a clock what
we want.
However, the host controller of both S5PC210 and S5PV310 don't have this so
we have to
Add additional routine to make a proper clock with outland clock divider
instead of
Internal clock divider in host controller.

Best Regards,
Jeongbae Seo

RE: [PATCH v2 2/2] sdhci-s3c: Add support no internal clock divider in host controller

[Kukjin Kim]

Jeongbae Seo wrote:
> 
> Ben Dooks worte:
> > On 17/09/10 10:45, Kukjin Kim wrote:
> > > From: Hyuk Lee <hyuk1.lee@samsung.com>
> > >
> > > This patch adds to support no internal clock divider in SDHCI.
> > > The external clock divider can be used to make a proper clock
> > > because SDHCI doesn't support internal clock divider by itself.
> > >
> > > Signed-off-by: Hyuk Lee <hyuk1.lee@samsung.com>
> > > Signed-off-by: Jeongbae Seo <jeongbae.seo@samsung.com>
> > > Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> > > ---
> > >  drivers/mmc/host/sdhci-s3c.c |   60
> > ++++++++++++++++++++++++++++++++++++++++++
> > >  1 files changed, 60 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/mmc/host/sdhci-s3c.c
b/drivers/mmc/host/sdhci-s3c.c
> > > index e6e0438..5ad5ed7 100644
> > > --- a/drivers/mmc/host/sdhci-s3c.c
> > > +++ b/drivers/mmc/host/sdhci-s3c.c
> > > @@ -96,6 +96,13 @@ static unsigned int sdhci_s3c_get_max_clk(struct
> > sdhci_host *host)
> > >  	unsigned int rate, max;
> > >  	int clk;
> > >
> > > +	/*
> > > +	 * There is only one clock source(sclk) if there is no clock
> > divider
> > > +	 * in the host controller
> > > +	 */
> > > +	if (host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK)
> > > +		return clk_round_rate(ourhost->clk_bus[2], UINT_MAX);
> >
> > interesting, <linux/clk.h> doesn't have a second parameter to
> > clk_round_rate().
> >
> Hi Ben,
> 
> Thanks for your comments.
> 
> When I see clk_round_rate in <linux/clk.h>, which function is defined
> as "long clk_round_rate(struct clk *clk, unsigned long rate)" that has two
> parameters.
> Please let me know if you have another meaning for this.
> 
> >
> > >  	/* note, a reset will reset the clock source */
> > >
> > >  	sdhci_s3c_check_sclk(host);
> > > @@ -130,6 +137,15 @@ static unsigned int
sdhci_s3c_consider_clock(struct
> > sdhci_s3c *ourhost,
> > >  	if (!clksrc)
> > >  		return UINT_MAX;
> > >
> > > +	/*
> > > +	 * There is only one clock source(sclk) if there is no clock
> > divider
> > > +	 * in the host controller
> > > +	 */
> > > +	if (ourhost->host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK) {
> > > +		rate = clk_round_rate(clksrc, wanted);
> > > +		return wanted - rate;
> > > +	}
> >
> > Why does this need a quirk, instead of just having one clock available
> > in the list of usable clocks?
> >
> 
> The available clock is made by dividing a clock source with a certain
> divider value.
> Most of the host controller has this capability that can divide a clock
what
> we want.
> However, the host controller of both S5PC210 and S5PV310 don't have this
so
> we have to
> Add additional routine to make a proper clock with outland clock divider
> instead of
> Internal clock divider in host controller.
> 
Hi Ben,

If any your suggestion in this case, S5PV310/S5PC210 has no clock divider in
the HSMMC IP block, please kindly let me know. Will address comments from
you.

As you know, there are many solution for this...But I'd like to use your
preference in here.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH v2 2/2] sdhci-s3c: Add support no internal clock divider in host controller

[Kukjin Kim]

Jeongbae Seo wrote:
> 
> Ben Dooks worte:
> > On 17/09/10 10:45, Kukjin Kim wrote:
> > > From: Hyuk Lee <hyuk1.lee@samsung.com>
> > >
> > > This patch adds to support no internal clock divider in SDHCI.
> > > The external clock divider can be used to make a proper clock
> > > because SDHCI doesn't support internal clock divider by itself.
> > >
> > > Signed-off-by: Hyuk Lee <hyuk1.lee@samsung.com>
> > > Signed-off-by: Jeongbae Seo <jeongbae.seo@samsung.com>
> > > Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> > > ---
> > >  drivers/mmc/host/sdhci-s3c.c |   60
> > ++++++++++++++++++++++++++++++++++++++++++
> > >  1 files changed, 60 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/mmc/host/sdhci-s3c.c
b/drivers/mmc/host/sdhci-s3c.c
> > > index e6e0438..5ad5ed7 100644
> > > --- a/drivers/mmc/host/sdhci-s3c.c
> > > +++ b/drivers/mmc/host/sdhci-s3c.c
> > > @@ -96,6 +96,13 @@ static unsigned int sdhci_s3c_get_max_clk(struct
> > sdhci_host *host)
> > >  	unsigned int rate, max;
> > >  	int clk;
> > >
> > > +	/*
> > > +	 * There is only one clock source(sclk) if there is no clock
> > divider
> > > +	 * in the host controller
> > > +	 */
> > > +	if (host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK)
> > > +		return clk_round_rate(ourhost->clk_bus[2], UINT_MAX);
> >
> > interesting, <linux/clk.h> doesn't have a second parameter to
> > clk_round_rate().
> >
> Hi Ben,
> 
> Thanks for your comments.
> 
> When I see clk_round_rate in <linux/clk.h>, which function is defined
> as "long clk_round_rate(struct clk *clk, unsigned long rate)" that has two
> parameters.
> Please let me know if you have another meaning for this.
> 
> >
> > >  	/* note, a reset will reset the clock source */
> > >
> > >  	sdhci_s3c_check_sclk(host);
> > > @@ -130,6 +137,15 @@ static unsigned int
sdhci_s3c_consider_clock(struct
> > sdhci_s3c *ourhost,
> > >  	if (!clksrc)
> > >  		return UINT_MAX;
> > >
> > > +	/*
> > > +	 * There is only one clock source(sclk) if there is no clock
> > divider
> > > +	 * in the host controller
> > > +	 */
> > > +	if (ourhost->host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK) {
> > > +		rate = clk_round_rate(clksrc, wanted);
> > > +		return wanted - rate;
> > > +	}
> >
> > Why does this need a quirk, instead of just having one clock available
> > in the list of usable clocks?
> >
> 
> The available clock is made by dividing a clock source with a certain
> divider value.
> Most of the host controller has this capability that can divide a clock
what
> we want.
> However, the host controller of both S5PC210 and S5PV310 don't have this
so
> we have to
> Add additional routine to make a proper clock with outland clock divider
> instead of
> Internal clock divider in host controller.
> 
Hi Ben,

If any your suggestion in this case, S5PV310/S5PC210 has no clock divider in
the HSMMC IP block, please kindly let me know. Will address comments from
you.

As you know, there are many solution for this...But I'd like to use your
preference in here.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.