Hi,
Wolfram, do you have time to look at this? Looks like we need to expose
suspend/resume hooks to -pltfm users for this use case -- I don't think
any esdhc code should be in sdhci-pltfm.c. (I don't mind writing the
patch if you agree that that's the correct solution here.)
Jerry, some style comments below.
Thanks,
- Chris.
On Mon, Dec 26 2011, r66093@xxxxxxxxxxxxx wrote:
> From: Jerry Huang <Chang-Ming.Huang@xxxxxxxxxxxxx>
>
> For FSL ESDHC controllor, when enter the sleep, the controller will power off,
controller, entering sleep,
> therefore the register will lost its valuse, and driver should save value of
lose its values,
> register during suspend and used during resume.
registers restore them during resume.
>
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@xxxxxxxxxxxxx>
> Signed-off-by: Jiang Yutang <b14898@xxxxxxxxxxxxx>
> CC: Chris Ball <cjb@xxxxxxxxxx>
> ---
> changes for v2:
> - change the property to compatible for quirks
> changes for v3:
> - fix one compile error
>
> drivers/mmc/host/sdhci-pltfm.c | 21 ++++++++++++++++++++-
> 1 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
> index 1c254b1..6791a2a 100644
> --- a/drivers/mmc/host/sdhci-pltfm.c
> +++ b/drivers/mmc/host/sdhci-pltfm.c
> @@ -53,6 +53,10 @@ static bool sdhci_of_wp_inverted(struct device_node *np)
> #endif /* CONFIG_PPC */
> }
>
> +#ifdef CONFIG_PM
> +static int sdhc_pmsaveproctlreg;
> +static u32 esdhc_proctl;
> +#endif
> void sdhci_get_of_property(struct platform_device *pdev)
> {
> struct device_node *np = pdev->dev.of_node;
> @@ -79,6 +83,11 @@ void sdhci_get_of_property(struct platform_device *pdev)
> || of_device_is_compatible(np, "fsl,mpc8536-esdhc"))
> host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
>
> +#ifdef CONFIG_PM
> + if (of_device_is_compatible(np, "fsl,p1022-esdhc"))
> + sdhc_pmsaveproctlreg = 1;
> +#endif
> +
> clk = of_get_property(np, "clock-frequency", &size);
> if (clk && size == sizeof(*clk) && *clk)
> pltfm_host->clock = be32_to_cpup(clk);
error: patch failed: drivers/mmc/host/sdhci-pltfm.c:79
error: drivers/mmc/host/sdhci-pltfm.c: patch does not apply
Needs to be rebased against mmc-next. But wait until we figure out
what to do with this patch.
> @@ -206,15 +215,25 @@ int sdhci_pltfm_suspend(struct platform_device *dev, pm_message_t state)
> {
> struct sdhci_host *host = platform_get_drvdata(dev);
>
> + if (sdhc_pmsaveproctlreg == 1)
> + esdhc_proctl = sdhci_readl(host, SDHCI_HOST_CONTROL);
> +
You can skip the "== 1".
> return sdhci_suspend_host(host, state);
> }
> EXPORT_SYMBOL_GPL(sdhci_pltfm_suspend);
>
> int sdhci_pltfm_resume(struct platform_device *dev)
> {
> + int ret;
> struct sdhci_host *host = platform_get_drvdata(dev);
>
> - return sdhci_resume_host(host);
> + host->ops->enable_dma(host);
(The indentation looks incorrect here.)
You shouldn't be calling an ops pointer without testing it first.
And you haven't explained why you're calling this unconditionally
for *every* -pltfm driver. How do you know it doesn't break one
of them?
> +
> + ret = mmc_resume_host(host->mmc);
> + if (sdhc_pmsaveproctlreg == 1)
> + sdhci_writel(host, esdhc_proctl, SDHCI_HOST_CONTROL);
> +
> + return ret;
== 1 not needed.
> }
> EXPORT_SYMBOL_GPL(sdhci_pltfm_resume);
> #endif /* CONFIG_PM */
Thanks,
- Chris.
--
Chris Ball <cjb@xxxxxxxxxx> <http://printf.net/>
One Laptop Per Child
--
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
