Re: [PATCH 1/3] mmc: sdhci-s3c: Remove usage of clk_type member in platform data

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

 



Hi,

On Tue, Oct 4, 2011 at 4:28 PM, Thomas Abraham
<thomas.abraham@xxxxxxxxxx> wrote:
> SDHCI controllers on Exynos4 do not include the sdclk divider as per the
> sdhci controller specification. This case can be represented using the
> sdhci quirk SDHCI_QUIRK_NONSTANDARD_CLOCK instead of using an additional
> enum type definition 'clk_types'.
>
> Hence, usage of clk_type member in platform data is removed and the sdhci
> quirk is used. In addition to that, since this qurik is SoC specific,
> driver data is introduced to represent controllers on SoC's that require
> this quirk.
Right.
>
> Cc: Ben Dooks <ben-linux@xxxxxxxxx>
> Cc: Jeongbae Seo <jeongbae.seo@xxxxxxxxxxx>
> Signed-off-by: Thomas Abraham <thomas.abraham@xxxxxxxxxx>
> ---
> In the function 'sdhci_cmu_set_clock', the code that waits for a stable
> sdclk clock is replicated from 'sdhci_set_clock' function. This portion
> of code is replicated in sdhci-cns3xxx driver as well. This duplication
> could have been avoid by adding a new function in sdhci host controller
> driver that waits for stable sdclk and sdhci-s3c/sdhci-cns3xxx can be
> the users of the new function. If adding a new function in sdhci host
> controller driver is acceptable, an additional patch in this series
> can be added that creates the new function that waits for stable sdclk.
>
>  drivers/mmc/host/sdhci-s3c.c |   74 +++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 70 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index 82709b6..b29e734 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -53,6 +53,18 @@ struct sdhci_s3c {
>        struct clk              *clk_bus[MAX_BUS_CLK];
>  };
>
> +/**
> + * struct sdhci_s3c_driver_data - S3C SDHCI platform specific driver data
> + * @sdhci_quirks: sdhci host specific quirks.
> + *
> + * Specifies platform specific configuration of sdhci controller.
> + * Note: A structure for driver specific platform data is used for future
> + * expansion of its usage.
> + */
> +struct sdhci_s3c_drv_data {
> +       unsigned int    sdhci_quirks;
> +};
> +
>  static inline struct sdhci_s3c *to_s3c(struct sdhci_host *host)
>  {
>        return sdhci_priv(host);
> @@ -132,10 +144,10 @@ static unsigned int sdhci_s3c_consider_clock(struct sdhci_s3c *ourhost,
>                return UINT_MAX;
>
>        /*
> -        * Clock divider's step is different as 1 from that of host controller
> -        * when 'clk_type' is S3C_SDHCI_CLK_DIV_EXTERNAL.
> +        * If controller uses a non-standard clock division, find the best clock
> +        * speed possible with selected clock source and skip the division.
>         */
> -       if (ourhost->pdata->clk_type) {
> +       if (ourhost->host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK) {
>                rate = clk_round_rate(clksrc, wanted);
>                return wanted - rate;
>        }
> @@ -272,6 +284,8 @@ static unsigned int sdhci_cmu_get_min_clock(struct sdhci_host *host)
>  static void sdhci_cmu_set_clock(struct sdhci_host *host, unsigned int clock)
>  {
>        struct sdhci_s3c *ourhost = to_s3c(host);
> +       unsigned long timeout;
> +       u16 clk = 0;
>
>        /* don't bother if the clock is going off */
>        if (clock == 0)
> @@ -282,6 +296,25 @@ static void sdhci_cmu_set_clock(struct sdhci_host *host, unsigned int clock)
>        clk_set_rate(ourhost->clk_bus[ourhost->cur_clk], clock);
>
>        host->clock = clock;
> +
> +       clk = SDHCI_CLOCK_INT_EN;
> +       sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> +
> +       /* Wait max 20 ms */
> +       timeout = 20;
> +       while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
> +               & SDHCI_CLOCK_INT_STABLE)) {
> +               if (timeout == 0) {
> +                       printk(KERN_ERR "%s: Internal clock never "
> +                               "stabilised.\n", mmc_hostname(host->mmc));
> +                       return;
> +               }
> +               timeout--;
> +               mdelay(1);
> +       }
> +
> +       clk |= SDHCI_CLOCK_CARD_EN;
> +       sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>  }
>
>  /**
> @@ -382,9 +415,17 @@ static void sdhci_s3c_setup_card_detect_gpio(struct sdhci_s3c *sc)
>        }
>  }
>
> +static inline struct sdhci_s3c_drv_data *sdhci_s3c_get_driver_data(
> +                       struct platform_device *pdev)
> +{
> +       return (struct sdhci_s3c_drv_data *)
> +                       platform_get_device_id(pdev)->driver_data;
> +}
> +
>  static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
>  {
>        struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;
> +       struct sdhci_s3c_drv_data *drv_data;
>        struct device *dev = &pdev->dev;
>        struct sdhci_host *host;
>        struct sdhci_s3c *sc;
> @@ -414,6 +455,7 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
>                return PTR_ERR(host);
>        }
>
> +       drv_data = sdhci_s3c_get_driver_data(pdev);
>        sc = sdhci_priv(host);
>
>        sc->host = host;
> @@ -494,6 +536,8 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
>        /* Setup quirks for the controller */
>        host->quirks |= SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
>        host->quirks |= SDHCI_QUIRK_NO_HISPD_BIT;
> +       if (drv_data)
> +               host->quirks |= drv_data->sdhci_quirks;
>
>  #ifndef CONFIG_MMC_SDHCI_S3C_DMA
>
> @@ -534,7 +578,7 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
>         * If controller does not have internal clock divider,
>         * we can use overriding functions instead of default.
>         */
> -       if (pdata->clk_type) {
> +       if (host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK) {
>                sdhci_s3c_ops.set_clock = sdhci_cmu_set_clock;
>                sdhci_s3c_ops.get_min_clock = sdhci_cmu_get_min_clock;
>                sdhci_s3c_ops.get_max_clock = sdhci_cmu_get_max_clock;
> @@ -639,9 +683,31 @@ static int sdhci_s3c_resume(struct platform_device *dev)
>  #define sdhci_s3c_resume NULL
>  #endif
>
> +#if defined(CONFIG_CPU_EXYNOS4210) || defined(CONFIG_SOC_EXYNOS4212)
> +static struct sdhci_s3c_drv_data exynos4_sdhci_drv_data = {
> +       .sdhci_quirks = SDHCI_QUIRK_NONSTANDARD_CLOCK,
> +};
> +#define EXYNOS4_SDHCI_DRV_DATA ((kernel_ulong_t)&exynos4_sdhci_drv_data)
> +#else
> +#define EXYNOS4_SDHCI_DRV_DATA (kernel_ulong_t)NULL
> +#endif
It's too small to save the memory. I think no need to guard the exynos
driver data with ifdef/endif.
> +
> +static struct platform_device_id sdhci_s3c_driver_ids[] = {
> +       {
> +               .name           = "s3c-sdhci",
> +               .driver_data    = (kernel_ulong_t)NULL,
> +       },
> +       {
> +               .name           = "exynos4-sdhci",
> +               .driver_data    = EXYNOS4_SDHCI_DRV_DATA,
> +       },
> +};
> +MODULE_DEVICE_TABLE(platform, sdhci_s3c_driver_ids);

Do you have a plan to use the sdhci-pltfm.c as other drivers did?
some drivers are already use it and support OF features.

Thank you,
Kyungmin Park
> +
>  static struct platform_driver sdhci_s3c_driver = {
>        .probe          = sdhci_s3c_probe,
>        .remove         = __devexit_p(sdhci_s3c_remove),
> +       .id_table       = sdhci_s3c_driver_ids,
>        .suspend        = sdhci_s3c_suspend,
>        .resume         = sdhci_s3c_resume,
>        .driver         = {
> --
> 1.6.6.rc2
>
> --
> 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
>
--
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