Hello, On Wednesday, July 28, 2010 6:48 PM Ben Dooks wrote: > On 28/07/10 15:19, Marek Szyprowski wrote: > > S3C SDHCI host controller can change the source for generating mmc clock. > > By default host bus clock is used, what causes some problems on machines > > with 133MHz bus, because the SDHCI divider cannot be as high get proper > > clock value for identification mode. This is not a problem for the > > controller, because it can generate lower frequencies from other clock > > sources. This patch adds a new quirk to SDHCI driver to calculate the > > minimal supported clock frequency. > > > > This fixes the flood of the following warnings on Samsung S5PV210 SoCs: > > mmc0: Minimum clock frequency too high for identification mode > > > > Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> > > Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx> > > --- > > > > Changes since V1: > > - rebased onto latest -mm kernel tree > > > > --- > > drivers/mmc/host/sdhci-of-esdhc.c | 1 + > > drivers/mmc/host/sdhci-s3c.c | 29 +++++++++++++++++++++++++++++ > > drivers/mmc/host/sdhci.c | 2 +- > > drivers/mmc/host/sdhci.h | 2 ++ > > 4 files changed, 33 insertions(+), 1 deletions(-) > > > > diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci- > of-esdhc.c > > index e9f99fe..763b364 100644 > > --- a/drivers/mmc/host/sdhci-of-esdhc.c > > +++ b/drivers/mmc/host/sdhci-of-esdhc.c > > @@ -124,6 +124,7 @@ struct sdhci_of_data sdhci_esdhc = { > > SDHCI_QUIRK_BROKEN_CARD_DETECTION | > > SDHCI_QUIRK_NO_BUSY_IRQ | > > SDHCI_QUIRK_NONSTANDARD_CLOCK | > > + SDHCI_QUIRK_NONSTANDARD_MINCLOCK | > > SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | > > SDHCI_QUIRK_PIO_NEEDS_DELAY | > > SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET | > > diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c > > index d7058ee..5218f2b 100644 > > --- a/drivers/mmc/host/sdhci-s3c.c > > +++ b/drivers/mmc/host/sdhci-s3c.c > > @@ -203,9 +203,36 @@ static void sdhci_s3c_set_clock(struct sdhci_host > *host, unsigned int clock) > > } > > } > > > > +/** > > + * sdhci_s3c_get_min_clock - callback to get minimal supported clock > value > > + * @host: The SDHCI host being queried > > + * > > + * To init mmc host properly a minimal clock value is needed. For high > system > > + * bus clock's values the standard formula gives values out of allowed > range. > > + * The clock still can be set to lower values, if clock source other > then > > + * system bus is selected. > > +*/ > > +static unsigned int sdhci_s3c_get_min_clock(struct sdhci_host *host) > > +{ > > + struct sdhci_s3c *ourhost = to_s3c(host); > > + unsigned int delta, min = UINT_MAX; > > + int src; > > + > > + for (src = 0; src < MAX_BUS_CLK; src++) { > > + delta = sdhci_s3c_consider_clock(ourhost, src, 0); > > + if (delta == UINT_MAX) > > + continue; > > + /* delta is a negative value in this case */ > > + if (-delta < min) > > + min = -delta; > > + } > > + return min; > > +} > > + > > static struct sdhci_ops sdhci_s3c_ops = { > > .get_max_clock = sdhci_s3c_get_max_clk, > > .set_clock = sdhci_s3c_set_clock, > > + .get_min_clock = sdhci_s3c_get_min_clock, > > }; > > > > static int __devinit sdhci_s3c_probe(struct platform_device *pdev) > > @@ -309,6 +336,8 @@ static int __devinit sdhci_s3c_probe(struct > platform_device *pdev) > > host->quirks = 0; > > host->irq = irq; > > > > + host->quirks |= SDHCI_QUIRK_NONSTANDARD_MINCLOCK; > > + > > /* Setup quirks for the controller */ > > host->quirks |= SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC; > > > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > > index 07b2695..682b285 100644 > > --- a/drivers/mmc/host/sdhci.c > > +++ b/drivers/mmc/host/sdhci.c > > @@ -1776,7 +1776,7 @@ int sdhci_add_host(struct sdhci_host *host) > > * Set host parameters. > > */ > > mmc->ops = &sdhci_ops; > > - if (host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK && > > + if (host->quirks & SDHCI_QUIRK_NONSTANDARD_MINCLOCK && > > host->ops->get_min_clock) > > mmc->f_min = host->ops->get_min_clock(host); > > We're begining to run out of quirk space, given these fields will > be initialised to NULL if not used, then why not just check for > the op being present? Ok, I can remove the quirk and just check if the op is present. However having a separate quirk makes the code easier to understand (quirk shows that the particular implementation differs from the standard). Best regards -- Marek Szyprowski Samsung Poland R&D Center -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html