* Re: [PATCH 2/2] mmc: sdhci-of-at91: Fix module autoload
From: Adrian Hunter @ 2016-10-21 7:23 UTC (permalink / raw)
To: Javier Martinez Canillas, linux-kernel, linux-mmc, Ulf Hansson
In-Reply-To: <20161021064819.4y5jrdetrqsvibfa@rfolt0960.corp.atmel.com>
On 21/10/16 09:48, Ludovic Desroches wrote:
> On Mon, Oct 17, 2016 at 01:13:45PM -0300, Javier Martinez Canillas wrote:
>> If the driver is built as a module, autoload won't work because the module
>> alias information is not filled. So user-space can't match the registered
>> device with the corresponding module.
>>
>> Export the module alias information using the MODULE_DEVICE_TABLE() macro.
>>
>> Before this patch:
>>
>> $ modinfo drivers/mmc/host/sdhci-of-at91.ko | grep alias
>> $
>>
>> After this patch:
>>
>> $ modinfo drivers/mmc/host/sdhci-of-at91.ko | grep alias
>> alias: of:N*T*Catmel,sama5d2-sdhciC*
>> alias: of:N*T*Catmel,sama5d2-sdhci
>>
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>
> Thanks
>> ---
>>
>> drivers/mmc/host/sdhci-of-at91.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
>> index a9b7fc06c434..2f9ad213377a 100644
>> --- a/drivers/mmc/host/sdhci-of-at91.c
>> +++ b/drivers/mmc/host/sdhci-of-at91.c
>> @@ -100,6 +100,7 @@ static const struct of_device_id sdhci_at91_dt_match[] = {
>> { .compatible = "atmel,sama5d2-sdhci", .data = &soc_data_sama5d2 },
>> {}
>> };
>> +MODULE_DEVICE_TABLE(of, sdhci_at91_dt_match);
>>
>> #ifdef CONFIG_PM
>> static int sdhci_at91_runtime_suspend(struct device *dev)
>> --
>> 2.7.4
>>
>
^ permalink raw reply
* Re: [PATCH 2/2] mmc: sdhci-of-at91: Fix module autoload
From: Ludovic Desroches @ 2016-10-21 6:48 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: linux-kernel, Ludovic Desroches, linux-mmc, Adrian Hunter,
Ulf Hansson
In-Reply-To: <1476720825-30442-2-git-send-email-javier@osg.samsung.com>
On Mon, Oct 17, 2016 at 01:13:45PM -0300, Javier Martinez Canillas wrote:
> If the driver is built as a module, autoload won't work because the module
> alias information is not filled. So user-space can't match the registered
> device with the corresponding module.
>
> Export the module alias information using the MODULE_DEVICE_TABLE() macro.
>
> Before this patch:
>
> $ modinfo drivers/mmc/host/sdhci-of-at91.ko | grep alias
> $
>
> After this patch:
>
> $ modinfo drivers/mmc/host/sdhci-of-at91.ko | grep alias
> alias: of:N*T*Catmel,sama5d2-sdhciC*
> alias: of:N*T*Catmel,sama5d2-sdhci
>
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
Thanks
> ---
>
> drivers/mmc/host/sdhci-of-at91.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
> index a9b7fc06c434..2f9ad213377a 100644
> --- a/drivers/mmc/host/sdhci-of-at91.c
> +++ b/drivers/mmc/host/sdhci-of-at91.c
> @@ -100,6 +100,7 @@ static const struct of_device_id sdhci_at91_dt_match[] = {
> { .compatible = "atmel,sama5d2-sdhci", .data = &soc_data_sama5d2 },
> {}
> };
> +MODULE_DEVICE_TABLE(of, sdhci_at91_dt_match);
>
> #ifdef CONFIG_PM
> static int sdhci_at91_runtime_suspend(struct device *dev)
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v5] mmc: sdhci-msm: Add pm_runtime and system PM support
From: Pramod Gurav @ 2016-10-21 6:42 UTC (permalink / raw)
To: ulf.hansson, adrian.hunter, linux-mmc
Cc: linux-kernel, linux-arm-msm, linux-pm, riteshh, Pramod Gurav
Provides runtime PM callbacks to enable and disable clock resources
when idle. Also support system PM callbacks to be called during system
suspend and resume.
Reviewed-by: Ritesh Harjani <riteshh@codeaurora.org>
Reviewed-by: Georgi Djakov <georgi.djakov@linaro.org>
Tested-by: Ritesh Harjani <riteshh@codeaurora.org>
Signed-off-by: Pramod Gurav <pramod.gurav@linaro.org>
---
Tested on DB410C.
Changes in v5:
- Added pm_runtime_mark_last_busy in probe before calling autosuspend
- included clock names in error logs
- Used micro instead of constant for autosuspend delay
- Removed platform_set_drvdata in probe as sdhci_pltfm_init does it.
- Aligned PM ops structure with the parenthesis
Changes in v4:
- Remove calls to sdhci_runtime_resume_host/sdhci_runtime_suspend_host
from runtime callbacks as sdhc msm controller is capable of restoring
it's register values after clocks are disabled and re-enabled.
Changes in v3:
- Added CONFIG_PM around runtime pm function.
- Replaced msm suspend/resume with generic function directly
- Use SET_SYSTEM_SLEEP_PM_OPS instead of late version
Changes in v2:
- Moved pm_rutime enabling before adding host
- Handled pm_rutime in remove
- Changed runtime handling with reference from sdhci-of-at91.c
drivers/mmc/host/sdhci-msm.c | 68 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 67 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 8ef44a2a..795f16f 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -18,6 +18,7 @@
#include <linux/of_device.h>
#include <linux/delay.h>
#include <linux/mmc/mmc.h>
+#include <linux/pm_runtime.h>
#include <linux/slab.h>
#include "sdhci-pltfm.h"
@@ -68,6 +69,7 @@
#define CMUX_SHIFT_PHASE_SHIFT 24
#define CMUX_SHIFT_PHASE_MASK (7 << CMUX_SHIFT_PHASE_SHIFT)
+#define MSM_MMC_AUTOSUSPEND_DELAY_MS 50
struct sdhci_msm_host {
struct platform_device *pdev;
void __iomem *core_mem; /* MSM SDCC mapped address */
@@ -658,12 +660,26 @@ static int sdhci_msm_probe(struct platform_device *pdev)
goto clk_disable;
}
+ pm_runtime_get_noresume(&pdev->dev);
+ pm_runtime_set_active(&pdev->dev);
+ pm_runtime_enable(&pdev->dev);
+ pm_runtime_set_autosuspend_delay(&pdev->dev,
+ MSM_MMC_AUTOSUSPEND_DELAY_MS);
+ pm_runtime_use_autosuspend(&pdev->dev);
+
ret = sdhci_add_host(host);
if (ret)
- goto clk_disable;
+ goto pm_runtime_disable;
+
+ pm_runtime_mark_last_busy(&pdev->dev);
+ pm_runtime_put_autosuspend(&pdev->dev);
return 0;
+pm_runtime_disable:
+ pm_runtime_disable(&pdev->dev);
+ pm_runtime_set_suspended(&pdev->dev);
+ pm_runtime_put_noidle(&pdev->dev);
clk_disable:
clk_disable_unprepare(msm_host->clk);
pclk_disable:
@@ -685,6 +701,11 @@ static int sdhci_msm_remove(struct platform_device *pdev)
0xffffffff);
sdhci_remove_host(host, dead);
+
+ pm_runtime_get_sync(&pdev->dev);
+ pm_runtime_disable(&pdev->dev);
+ pm_runtime_put_noidle(&pdev->dev);
+
clk_disable_unprepare(msm_host->clk);
clk_disable_unprepare(msm_host->pclk);
if (!IS_ERR(msm_host->bus_clk))
@@ -693,12 +714,57 @@ static int sdhci_msm_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_PM
+static int sdhci_msm_runtime_suspend(struct device *dev)
+{
+ struct sdhci_host *host = dev_get_drvdata(dev);
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+
+ clk_disable_unprepare(msm_host->clk);
+ clk_disable_unprepare(msm_host->pclk);
+
+ return 0;
+}
+
+static int sdhci_msm_runtime_resume(struct device *dev)
+{
+ struct sdhci_host *host = dev_get_drvdata(dev);
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+ int ret;
+
+ ret = clk_prepare_enable(msm_host->clk);
+ if (ret) {
+ dev_err(dev, "clk_enable failed for core_clk: %d\n", ret);
+ return ret;
+ }
+ ret = clk_prepare_enable(msm_host->pclk);
+ if (ret) {
+ dev_err(dev, "clk_enable failed for iface_clk: %d\n", ret);
+ clk_disable_unprepare(msm_host->clk);
+ return ret;
+ }
+
+ return 0;
+}
+#endif
+
+static const struct dev_pm_ops sdhci_msm_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
+ SET_RUNTIME_PM_OPS(sdhci_msm_runtime_suspend,
+ sdhci_msm_runtime_resume,
+ NULL)
+};
+
static struct platform_driver sdhci_msm_driver = {
.probe = sdhci_msm_probe,
.remove = sdhci_msm_remove,
.driver = {
.name = "sdhci_msm",
.of_match_table = sdhci_msm_dt_match,
+ .pm = &sdhci_msm_pm_ops,
},
};
--
2.9.3
^ permalink raw reply related
* Re: [PATCH] mmc: block: Change MMC_IOC_MAX_BYTES
From: Jaehoon Chung @ 2016-10-21 6:38 UTC (permalink / raw)
To: Jeremy Kim, linux-mmc; +Cc: ulf.hansson
In-Reply-To: <0d4301d229fc$607a9770$216fc650$@samsung.com>
Hi,
On 10/19/2016 08:31 PM, Jeremy Kim wrote:
>>From 27d46f5697434542ad9cafbc6a9630dc14915f91 Mon Sep 17 00:00:00 2001
> From: Jeonghan Kim <jh4u.kim@samsung.com>
> Date: Wed, 19 Oct 2016 19:48:02 +0900
> Subject: [PATCH] mmc: block: Change MMC_IOC_MAX_BYTES
>
> It is used for limitation of buffer size during IOCTL such as FFU.
> However, eMMC FW size is bigger than (512L*256).
> (For instance, currently, Samsung eMMC FW size is over 300KB.)
> So, it needs to increase to execute FFU.
>
> Signed-off-by: Jeonghan Kim <jh4u.kim@samsung.com>
Looks good to me.
Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
Best Regards,
Jaehoon Chung
> ---
> include/uapi/linux/mmc/ioctl.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/mmc/ioctl.h b/include/uapi/linux/mmc/ioctl.h
> index 7e385b8..700a551 100644
> --- a/include/uapi/linux/mmc/ioctl.h
> +++ b/include/uapi/linux/mmc/ioctl.h
> @@ -69,6 +69,6 @@ struct mmc_ioc_multi_cmd {
> * is enforced per ioctl call. For larger data transfers, use the normal
> * block device operations.
> */
> -#define MMC_IOC_MAX_BYTES (512L * 256)
> +#define MMC_IOC_MAX_BYTES (512L * 1024)
> #define MMC_IOC_MAX_CMDS 255
> #endif /* LINUX_MMC_IOCTL_H */
> --
> 1.9.1
>
>
>
>
^ permalink raw reply
* Re: [PATCH v4] mmc: sdhci-msm: Add pm_runtime and system PM support
From: Pramod Gurav @ 2016-10-21 4:22 UTC (permalink / raw)
To: Ritesh Harjani
Cc: Ulf Hansson, Adrian Hunter, linux-mmc, open list,
open list:ARM/QUALCOMM SUPPORT, linux-pm
In-Reply-To: <d7801576-e2b3-13a3-df8d-e203a628a445@codeaurora.org>
Hi Ritesh,
On 20 October 2016 at 20:20, Ritesh Harjani <riteshh@codeaurora.org> wrote:
> Hi Pramod,
>
> Thanks for this patch. Few minor comments.
>
> I have tested your patch on db410c and 8996 based internal platform and it
> works fine.
Thanks for the review and testing.
>
>
>
> On 10/18/2016 3:46 PM, Pramod Gurav wrote:
>>
>> Provides runtime PM callbacks to enable and disable clock resources
>> when idle. Also support system PM callbacks to be called during system
>> suspend and resume.
>>
>> Signed-off-by: Pramod Gurav <pramod.gurav@linaro.org>
>> ---
>>
>> Tested on DB410C.
>>
>> Changes in v4:
>> - Remove calls to sdhci_runtime_resume_host/sdhci_runtime_suspend_host
>> from runtime callbacks as sdhc msm controller is capable of restoring
>> it's register values after clocks are disabled and re-enabled.
>>
>> Changes in v3:
>> - Added CONFIG_PM around runtime pm function.
>> - Replaced msm suspend/resume with generic function directly
>> - Use SET_SYSTEM_SLEEP_PM_OPS instead of late version
>>
>> Changes in v2:
>> - Moved pm_rutime enabling before adding host
>> - Handled pm_rutime in remove
>> - Changed runtime handling with reference from sdhci-of-at91.c
>>
>> drivers/mmc/host/sdhci-msm.c | 66
>> +++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 65 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 8ef44a2a..33ec809 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -18,6 +18,7 @@
>> #include <linux/of_device.h>
>> #include <linux/delay.h>
>> #include <linux/mmc/mmc.h>
>> +#include <linux/pm_runtime.h>
>> #include <linux/slab.h>
>>
>> #include "sdhci-pltfm.h"
>> @@ -658,12 +659,26 @@ static int sdhci_msm_probe(struct platform_device
>> *pdev)
>> goto clk_disable;
>> }
>>
>> + pm_runtime_get_noresume(&pdev->dev);
>> + pm_runtime_set_active(&pdev->dev);
>> + pm_runtime_enable(&pdev->dev);
>> + pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
>
> Use a macro MSM_MMC_AUTOSUSPEND_DELAY_MS instead of using 50 directly here.
Will add.
>
>> + pm_runtime_use_autosuspend(&pdev->dev);
>> +
>> ret = sdhci_add_host(host);
>> if (ret)
>> - goto clk_disable;
>> + goto pm_runtime_disable;
>> +
>> + platform_set_drvdata(pdev, host);
>
> No need to set platform_set_drvdata here. sdhci_pltfm_init will do it
> anyways.
Good to know this. Will do away with this.
>
>> +
>
> pm_runtime_mark_last_busy(&pdev->dev) can be added here.
Yeah. Agree.
>
>> + pm_runtime_put_autosuspend(&pdev->dev);
>>
>> return 0;
>>
>> +pm_runtime_disable:
>> + pm_runtime_disable(&pdev->dev);
>> + pm_runtime_set_suspended(&pdev->dev);
>> + pm_runtime_put_noidle(&pdev->dev);
>> clk_disable:
>> clk_disable_unprepare(msm_host->clk);
>> pclk_disable:
>> @@ -685,6 +700,11 @@ static int sdhci_msm_remove(struct platform_device
>> *pdev)
>> 0xffffffff);
>>
>> sdhci_remove_host(host, dead);
>> +
>> + pm_runtime_get_sync(&pdev->dev);
>> + pm_runtime_disable(&pdev->dev);
>> + pm_runtime_put_noidle(&pdev->dev);
>> +
>> clk_disable_unprepare(msm_host->clk);
>> clk_disable_unprepare(msm_host->pclk);
>> if (!IS_ERR(msm_host->bus_clk))
>> @@ -693,12 +713,56 @@ static int sdhci_msm_remove(struct platform_device
>> *pdev)
>> return 0;
>> }
>>
>> +#ifdef CONFIG_PM
>> +static int sdhci_msm_runtime_suspend(struct device *dev)
>> +{
>> + struct sdhci_host *host = dev_get_drvdata(dev);
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +
>> + clk_disable_unprepare(msm_host->clk);
>> + clk_disable_unprepare(msm_host->pclk);
>> +
>> + return 0;
>> +}
>> +
>> +static int sdhci_msm_runtime_resume(struct device *dev)
>> +{
>> + struct sdhci_host *host = dev_get_drvdata(dev);
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> + int ret;
>> +
>> + ret = clk_prepare_enable(msm_host->clk);
>> + if (ret) {
>> + dev_err(dev, "clk_enable failed: %d\n", ret);
>
> dev_err(dev, "clk enable failed for core_clk: %d\n", ret);
>
>> + return ret;
>> + }
>> + ret = clk_prepare_enable(msm_host->pclk);
>> + if (ret) {
>> + dev_err(dev, "clk_enable failed: %d\n", ret);
>
> dev_err(dev, "clk enable failed for iface_clk: %d\n", ret);
>
Yes. will do these changes. Thanks for comments. :)
>> + clk_disable_unprepare(msm_host->clk);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +#endif
>> +
>> +static const struct dev_pm_ops sdhci_msm_pm_ops = {
>> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>> + pm_runtime_force_resume)
>> + SET_RUNTIME_PM_OPS(sdhci_msm_runtime_suspend,
>> sdhci_msm_runtime_resume,
>> + NULL)
>> +};
>> +
>> static struct platform_driver sdhci_msm_driver = {
>> .probe = sdhci_msm_probe,
>> .remove = sdhci_msm_remove,
>> .driver = {
>> .name = "sdhci_msm",
>> .of_match_table = sdhci_msm_dt_match,
>> + .pm = &sdhci_msm_pm_ops,
>> },
>> };
>>
>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v4] mmc: sdhci-msm: Add pm_runtime and system PM support
From: Pramod Gurav @ 2016-10-21 4:18 UTC (permalink / raw)
To: Georgi Djakov
Cc: Ulf Hansson, Adrian Hunter, linux-mmc, open list,
open list:ARM/QUALCOMM SUPPORT, linux-pm, Ritesh Harjani
In-Reply-To: <e203153c-da0a-d450-3471-9d0dabf0a2ed@linaro.org>
Thanks Georgi for the review.
On 20 October 2016 at 18:38, Georgi Djakov <georgi.djakov@linaro.org> wrote:
> Hi Pramod,
>
> Thanks for the patch!
>
> On 10/18/2016 01:16 PM, Pramod Gurav wrote:
>>
>> Provides runtime PM callbacks to enable and disable clock resources
>> when idle. Also support system PM callbacks to be called during system
>> suspend and resume.
>>
>> Signed-off-by: Pramod Gurav <pramod.gurav@linaro.org>
>> ---
>>
>> Tested on DB410C.
>>
>
> [..]
>
>> +static int sdhci_msm_runtime_resume(struct device *dev)
>> +{
>> + struct sdhci_host *host = dev_get_drvdata(dev);
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> + int ret;
>> +
>> + ret = clk_prepare_enable(msm_host->clk);
>> + if (ret) {
>> + dev_err(dev, "clk_enable failed: %d\n", ret);
>> + return ret;
>> + }
>> + ret = clk_prepare_enable(msm_host->pclk);
>> + if (ret) {
>> + dev_err(dev, "clk_enable failed: %d\n", ret);
>
>
> Nit: Maybe mention in the prints which clock failed - core or peripheral.
Agree. Will add in v5.
>
>> + clk_disable_unprepare(msm_host->clk);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +#endif
>> +
>> +static const struct dev_pm_ops sdhci_msm_pm_ops = {
>> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>> + pm_runtime_force_resume)
>
>
> Nit: Please align with the parenthesis.
>
>> + SET_RUNTIME_PM_OPS(sdhci_msm_runtime_suspend,
>> sdhci_msm_runtime_resume,
>> + NULL)
>
>
> Ditto.
Yes. Will take care of this as well.
>
> Reviewed-by: Georgi Djakov <georgi.djakov@linaro.org>
Thanks again. :)
>
> BR,
> Georgi
^ permalink raw reply
* Re: [PATCH v2] mmc: sunxi: Prevent against null dereference for vmmc
From: Chen-Yu Tsai @ 2016-10-21 2:52 UTC (permalink / raw)
To: Maxime Ripard
Cc: Ulf Hansson, linux-mmc@vger.kernel.org, linux-kernel,
Hans de Goede, Chen-Yu Tsai, linux-arm-kernel
In-Reply-To: <20161019133304.22915-1-maxime.ripard@free-electrons.com>
On Wed, Oct 19, 2016 at 9:33 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> VMMC is an optional regulator, which means that mmc_regulator_get_supply
> will only return an error in case of a deferred probe, but not when the
> regulator is not set in the DT.
>
> However, the sunxi driver assumes that VMMC is always there, and doesn't
> check the value of the regulator pointer before using it, which obviously
> leads to a (close to) null pointer dereference.
>
> Add proper checks to prevent that.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Chen-Yu Tsai <wens@csie.org>
^ permalink raw reply
* Re: [PATCH v2 0/9] Init runtime PM support for dw_mmc
From: Shawn Lin @ 2016-10-21 2:43 UTC (permalink / raw)
To: Jaehoon Chung, Ulf Hansson
Cc: shawn.lin, linux-mmc, Doug Anderson,
open list:ARM/Rockchip SoC..., Heiko Stuebner
In-Reply-To: <c4461e79-a478-0412-fbe7-8b6c0b1e4824@samsung.com>
Hi Jaehoon,
On 2016/10/21 9:56, Jaehoon Chung wrote:
> Hi Shawn,
>
> On 10/18/2016 08:35 PM, Shawn Lin wrote:
>> 在 2016/10/18 16:46, Ulf Hansson 写道:
>>> + Heiko
>>>
>>> On 12 October 2016 at 04:50, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>>>>
>>>> Hi Jaehoon and Ulf,
>
> I applied this on my repository. If there are any issue, let me know.
> And minor issue can be fixed..step by step.
I was checking your repository and there is no doubt for them.
Thanks a lot!
>
> Best Regards,
> Jaehoon Chung
>
>>>>
>>>> This patch is gonna support runtime PM for dw_mmc.
>>>> It could support to disable ciu_clk by default and disable
>>>> biu_clk if the devices are non-removeable, or removeable
>>>> with gpio-base card detect.
>>>>
>>>> Then I remove the system PM since the runtime PM actually
>>>> does the same thing as it. So I help migrate the dw_mmc variant
>>>> drivers to use runtime PM pairs and pm_runtime_force_*. Note
>>>> that I only enable runtime PM for dw_mmc-rockchip as I will
>>>> leave the decision to the owners of the corresponding drivers.
>>>> I just tested it on my RK3288 platform with linux-next to make
>>>> the runtime PM and system PM work fine for my emmc, sd card and
>>>> sdio. But I don't have hardware to help test other variant drivers.
>>>> But in theory it should work fine as I mentioned that the runtime
>>>> PM does the same thing as system PM except for disabling ciu_clk
>>>> aggressively which should not be related to the variant hosts.
>>>>
>>>> As you could see that I just extend the slot-gpio a bit, so the
>>>> ideal way is Ulf could pick them up with Jaehoon's ack. :)
>>>
>>> The mmc core change looks fine to me, so I will wait for a pull
>>> request from Jaehoon.
>>>
>>>>
>>>>
>>>> Changes in v2:
>>>> - use struct device as argument for runtime callback
>>>> - use dw_mci_runtime_* directly
>>>> - use dw_mci_runtime_* directly
>>>> - minor fix since I change the argument for dw_mci_runtime_*
>>>> - use dw_mci_runtime_* directly
>>>> - use dw_mci_runtime_* directly
>>>>
>>>> Shawn Lin (9):
>>>> mmc: dw_mmc: add runtime PM callback
>>>> mmc: dw_mmc-rockchip: add runtime PM support
>>>> mmc: core: expose the capability of gpio card detect
>>>> mmc: dw_mmc: disable biu clk if possible
>>>> mmc: dw_mmc-k3: deploy runtime PM facilities
>>>> mmc: dw_mmc-exynos: deploy runtime PM facilities
>>>> mmc: dw_mmc-pci: deploy runtime PM facilities
>>>> mmc: dw_mmc-pltfm: deploy runtime PM facilities
>>>> mmc: dw_mmc: remove system PM callback
>>>>
>>>> drivers/mmc/core/slot-gpio.c | 8 +++++++
>>>> drivers/mmc/host/dw_mmc-exynos.c | 24 +++++++++-----------
>>>> drivers/mmc/host/dw_mmc-k3.c | 39 ++++++++------------------------
>>>> drivers/mmc/host/dw_mmc-pci.c | 29 ++++++++----------------
>>>> drivers/mmc/host/dw_mmc-pltfm.c | 28 +++++++----------------
>>>> drivers/mmc/host/dw_mmc-rockchip.c | 42 +++++++++++++++++++++++++++++++---
>>>> drivers/mmc/host/dw_mmc.c | 46 +++++++++++++++++++++++++-------------
>>>> drivers/mmc/host/dw_mmc.h | 6 ++---
>>>> include/linux/mmc/slot-gpio.h | 1 +
>>>> 9 files changed, 117 insertions(+), 106 deletions(-)
>>>>
>>>
>>> Overall these changes looks good to me, so I am ready to accept the PR
>>> from Jaehoon!!
>>>
>>>
>>> Although, highly related to this patchset, I am worried that there is
>>> a misunderstanding on how MMC_PM_KEEP_POWER (DT binding
>>> "keep-power-in-suspend") is being used for dw_mmc. Perhaps I am wrong,
>>> but I would appreciate if you could elaborate a bit for my
>>> understanding.
>>>
>>> First, this cap is solely intended to be used for controllers which
>>> may have SDIO cards attached, as it indicates those cards may be
>>> configured to be powered on while the system enters suspend state. By
>>> looking at some DTS files, for example
>>> arch/arm64/boot/dts/rockchip/rk3368-orion-r68-meta.dts which uses it
>>> for an eMMC slot, this is clearly being abused.
>>
>> Indeed. In general, it should be copy-paste issues as folks maybe write
>> their dts referring to the exist dts there. So yes, I will do some cleanup work for them in prevent of further spread of abused code.
>>
>>>
>>> Anyway, the wrong DT configurations might not be a big deal, as in
>>> dw_mci_resume(), it's not the capabilities bit that is checked but the
>>> corresponding "pm_flag". This flag is set via calling
>>> sdio_set_host_pm_flags(), but as that doesn't happen for an eMMC card
>>> we should be fine, right!?
>>>
>>> Now, what also do puzzles me, is exactly that piece of code in
>>> dw_mci_resume() that is being executed *when* the pm_flag contains
>>> MMC_PM_KEEP_POWER:
>>> if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
>>> dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
>>> dw_mci_setup_bus(slot, true);
>>> }
>>>
>>> So, in the system resume path, the above do makes sense as you need to
>>> restore the registers etc for the dw_mmc controller to enable it to
>>> operate the SDIO card. Such as bus width, clocks, etc.
>>>
>>> Although, I would expect something similar would be needed in the new
>>> runtime resume path as well. And in particular also for eMMC/SD cards,
>>> as you need to restore the dw_mmc registers to be able to operate the
>>> card again. Don't you?
>>
>> yes, we do.
>>
>>>
>>> So in the end, perhaps you should *always* call dw_mci_set_ios() and
>>> dw_mci_setup_bus() in dw_mci_resume() instead of conditionally check
>>> for MMC_PM_KEEP_POWER? Or maybe only a subset of those functions?
>>>
>>
>> Thanks for noticing this.
>>
>> Personally, I realize there is no need to check MMC_PM_KEEP_POWER but
>> it could be highly related to the cost of S-2-R, I guess. I just checked
>> sdhci and saw the similar cases you mentioned at the first glance.
>> Maybe I'm wrong but I need more time to investigate this issue later.
>>
>> There are still some on-going cleanup work for dw_mmc listed on my TODO
>> list, including bugfix, legacy/redundant code etc.. So I will check this one either. Maybe Jaehoon could also do some stree test on enxyos
>> platforms. :)
>>
>>
>>> Kind regards
>>> Uffe
>>>
>>>
>>>
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Best Regards
Shawn Lin
^ permalink raw reply
* Re: [PATCH v2 0/9] Init runtime PM support for dw_mmc
From: Jaehoon Chung @ 2016-10-21 1:56 UTC (permalink / raw)
To: Shawn Lin, Ulf Hansson
Cc: linux-mmc, Doug Anderson, open list:ARM/Rockchip SoC...,
Heiko Stuebner
In-Reply-To: <5be33ac5-7b5e-4b53-67bf-1c38822832ea@rock-chips.com>
Hi Shawn,
On 10/18/2016 08:35 PM, Shawn Lin wrote:
> 在 2016/10/18 16:46, Ulf Hansson 写道:
>> + Heiko
>>
>> On 12 October 2016 at 04:50, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>>>
>>> Hi Jaehoon and Ulf,
I applied this on my repository. If there are any issue, let me know.
And minor issue can be fixed..step by step.
Best Regards,
Jaehoon Chung
>>>
>>> This patch is gonna support runtime PM for dw_mmc.
>>> It could support to disable ciu_clk by default and disable
>>> biu_clk if the devices are non-removeable, or removeable
>>> with gpio-base card detect.
>>>
>>> Then I remove the system PM since the runtime PM actually
>>> does the same thing as it. So I help migrate the dw_mmc variant
>>> drivers to use runtime PM pairs and pm_runtime_force_*. Note
>>> that I only enable runtime PM for dw_mmc-rockchip as I will
>>> leave the decision to the owners of the corresponding drivers.
>>> I just tested it on my RK3288 platform with linux-next to make
>>> the runtime PM and system PM work fine for my emmc, sd card and
>>> sdio. But I don't have hardware to help test other variant drivers.
>>> But in theory it should work fine as I mentioned that the runtime
>>> PM does the same thing as system PM except for disabling ciu_clk
>>> aggressively which should not be related to the variant hosts.
>>>
>>> As you could see that I just extend the slot-gpio a bit, so the
>>> ideal way is Ulf could pick them up with Jaehoon's ack. :)
>>
>> The mmc core change looks fine to me, so I will wait for a pull
>> request from Jaehoon.
>>
>>>
>>>
>>> Changes in v2:
>>> - use struct device as argument for runtime callback
>>> - use dw_mci_runtime_* directly
>>> - use dw_mci_runtime_* directly
>>> - minor fix since I change the argument for dw_mci_runtime_*
>>> - use dw_mci_runtime_* directly
>>> - use dw_mci_runtime_* directly
>>>
>>> Shawn Lin (9):
>>> mmc: dw_mmc: add runtime PM callback
>>> mmc: dw_mmc-rockchip: add runtime PM support
>>> mmc: core: expose the capability of gpio card detect
>>> mmc: dw_mmc: disable biu clk if possible
>>> mmc: dw_mmc-k3: deploy runtime PM facilities
>>> mmc: dw_mmc-exynos: deploy runtime PM facilities
>>> mmc: dw_mmc-pci: deploy runtime PM facilities
>>> mmc: dw_mmc-pltfm: deploy runtime PM facilities
>>> mmc: dw_mmc: remove system PM callback
>>>
>>> drivers/mmc/core/slot-gpio.c | 8 +++++++
>>> drivers/mmc/host/dw_mmc-exynos.c | 24 +++++++++-----------
>>> drivers/mmc/host/dw_mmc-k3.c | 39 ++++++++------------------------
>>> drivers/mmc/host/dw_mmc-pci.c | 29 ++++++++----------------
>>> drivers/mmc/host/dw_mmc-pltfm.c | 28 +++++++----------------
>>> drivers/mmc/host/dw_mmc-rockchip.c | 42 +++++++++++++++++++++++++++++++---
>>> drivers/mmc/host/dw_mmc.c | 46 +++++++++++++++++++++++++-------------
>>> drivers/mmc/host/dw_mmc.h | 6 ++---
>>> include/linux/mmc/slot-gpio.h | 1 +
>>> 9 files changed, 117 insertions(+), 106 deletions(-)
>>>
>>
>> Overall these changes looks good to me, so I am ready to accept the PR
>> from Jaehoon!!
>>
>>
>> Although, highly related to this patchset, I am worried that there is
>> a misunderstanding on how MMC_PM_KEEP_POWER (DT binding
>> "keep-power-in-suspend") is being used for dw_mmc. Perhaps I am wrong,
>> but I would appreciate if you could elaborate a bit for my
>> understanding.
>>
>> First, this cap is solely intended to be used for controllers which
>> may have SDIO cards attached, as it indicates those cards may be
>> configured to be powered on while the system enters suspend state. By
>> looking at some DTS files, for example
>> arch/arm64/boot/dts/rockchip/rk3368-orion-r68-meta.dts which uses it
>> for an eMMC slot, this is clearly being abused.
>
> Indeed. In general, it should be copy-paste issues as folks maybe write
> their dts referring to the exist dts there. So yes, I will do some cleanup work for them in prevent of further spread of abused code.
>
>>
>> Anyway, the wrong DT configurations might not be a big deal, as in
>> dw_mci_resume(), it's not the capabilities bit that is checked but the
>> corresponding "pm_flag". This flag is set via calling
>> sdio_set_host_pm_flags(), but as that doesn't happen for an eMMC card
>> we should be fine, right!?
>>
>> Now, what also do puzzles me, is exactly that piece of code in
>> dw_mci_resume() that is being executed *when* the pm_flag contains
>> MMC_PM_KEEP_POWER:
>> if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
>> dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
>> dw_mci_setup_bus(slot, true);
>> }
>>
>> So, in the system resume path, the above do makes sense as you need to
>> restore the registers etc for the dw_mmc controller to enable it to
>> operate the SDIO card. Such as bus width, clocks, etc.
>>
>> Although, I would expect something similar would be needed in the new
>> runtime resume path as well. And in particular also for eMMC/SD cards,
>> as you need to restore the dw_mmc registers to be able to operate the
>> card again. Don't you?
>
> yes, we do.
>
>>
>> So in the end, perhaps you should *always* call dw_mci_set_ios() and
>> dw_mci_setup_bus() in dw_mci_resume() instead of conditionally check
>> for MMC_PM_KEEP_POWER? Or maybe only a subset of those functions?
>>
>
> Thanks for noticing this.
>
> Personally, I realize there is no need to check MMC_PM_KEEP_POWER but
> it could be highly related to the cost of S-2-R, I guess. I just checked
> sdhci and saw the similar cases you mentioned at the first glance.
> Maybe I'm wrong but I need more time to investigate this issue later.
>
> There are still some on-going cleanup work for dw_mmc listed on my TODO
> list, including bugfix, legacy/redundant code etc.. So I will check this one either. Maybe Jaehoon could also do some stree test on enxyos
> platforms. :)
>
>
>> Kind regards
>> Uffe
>>
>>
>>
>
>
^ permalink raw reply
* Re: [PATCH v3 2/3] mmc: tmio-mmc: add support for 32bit data port
From: Wolfram Sang @ 2016-10-20 23:12 UTC (permalink / raw)
To: Chris Brandt
Cc: Ulf Hansson, Wolfram Sang, Sergei Shtylyov, Geert Uytterhoeven,
Simon Horman, linux-mmc@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
In-Reply-To: <SG2PR06MB11652EB5B1A40583636F8FB28AD50@SG2PR06MB1165.apcprd06.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 712 bytes --]
> However, for version 3.0 that will be posted shortly, it will start to include the SDHI section.
Nice. Could you ping me when this comes out?
> > * Does it have a version register (CTL_VERSION)? If so, what does it
> > say?
>
> 0x820B
Okay, this is a version I have not seen before.
> > * Does it have SD_BUF0 width setting (named either EXT_ACC or HOST_MODE,
> > so far always comes after the version register)? If so, what is its
> > layout?
>
> There is a "EXT_SWAP" register. It is the only thing after VERSION.
> VERSION = 0xE804E0E2
> EXT_SWAP = 0xE804E0F0
This SWAP register exists on R-Car as well. Out of curiosity, what is the
register value of 0xE804E0E4?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v3 2/3] mmc: tmio-mmc: add support for 32bit data port
From: Wolfram Sang @ 2016-10-20 23:04 UTC (permalink / raw)
To: Chris Brandt
Cc: Ulf Hansson, Wolfram Sang, Sergei Shtylyov, Geert Uytterhoeven,
Simon Horman, linux-mmc@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
In-Reply-To: <SG2PR06MB1165B756B67D1D101E91A7B38AD50@SG2PR06MB1165.apcprd06.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 182 bytes --]
> I'll submit a v4 of this patch series tomorrow.
Please wait a little. I'd like to use that feature on R-Car, too, so I
wonder if the additional feature flag is the proper path.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* RE: [PATCH v3 2/3] mmc: tmio-mmc: add support for 32bit data port
From: Chris Brandt @ 2016-10-20 19:46 UTC (permalink / raw)
To: Wolfram Sang
Cc: Ulf Hansson, Wolfram Sang, Sergei Shtylyov, Geert Uytterhoeven,
Simon Horman, linux-mmc@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
In-Reply-To: <20161020132852.GB4177@katana>
Hello Wolfram,
> > > + /* if count was multiple of 4 */
> > > + if (!(count & 0x3))
> > > + return;
> > > + buf8 = (u8 *)(buf + (count >> 2));
> > > + count %= 4;
> >
> > To skip the same operation done on 'count' twice, maybe?
> >
> > buf8 = (u8 *)(buf + (count >> 2));
> > count &= 3;
> >
> > if (!count)
> > return;
>
>
> That looks like it should work.
> I can try it out and see if there is something we are missing.
Seems to work fine. Although I admit that because the driver is mostly being used as a block device (SD card), all the data requests are an even multiple of 4 (I put a printk after "if (!count) return;" ). Not sure if that will be the case when I test SDIO devices, but I'll find out once I start porting SDIO device drivers to 4.9+.
I'll submit a v4 of this patch series tomorrow.
Thank you.
Chris
^ permalink raw reply
* Re: Regression after "do not use CMD13 to get status after speed mode switch"
From: Srinivas Kandagatla @ 2016-10-20 15:17 UTC (permalink / raw)
To: Linus Walleij, linux-mmc@vger.kernel.org, Ulf Hansson,
Chaotian Jing
Cc: linux-arm-msm@vger.kernel.org, Bjorn Andersson, Stephen Boyd,
Andy Gross
In-Reply-To: <CACRpkdbfNq+R9U9AwsKBM=xyKKTzyVDkiW4jqJ0Nb2LXvq20BA@mail.gmail.com>
Hi Linus,
On 17/10/16 15:32, Linus Walleij wrote:
> Hi Chaotian,
>
> my Dragonboard 8060 has a regression on detecting the eMMC after
> bisecting it down to this patch (which is sadly already in v4.8):
>
> commit 08573eaf1a70104f83fdbee9b84e5be03480e9ed
> Author: Chaotian Jing <chaotian.jing@mediatek.com>
> Date: Thu May 19 16:47:41 2016 +0800
> mmc: mmc: do not use CMD13 to get status after speed mode switch
>
> Before this patch the eMMC is detected and all partitions enumerated
> immediately, but after the patch it doesn't come up at all, except
> sometimes, when it appears minutes (!) after boot, all of a sudden.
>
> Do you have any ideas on what can be wrong?
I remember hitting timeout issue with new version of eMMC on SD600c-eval
board, this patch/hack fixed it for me.
https://git.linaro.org/people/srinivas.kandagatla/linux.git/commitdiff/e42d7427b9ce77174eef079be0e04e22bdb8fd07?hp=3e4bf0b78405694fd1ab4371c73e80932be21e3a
Also one more important thing was to do with regulator mode too.
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=93b8e6fda11757a20df21f3fc18e12789841f173
Can you give it a try.. If it works I can clean it up and send it for
review.
Thanks,
srini
>
> This is the Qualcomm MMCI variant whid does *not* support
> busydetection in hardware.
>
> I will continue looking at it but would be happy for any hints on how to
> solve it, currently I cannot mount root off the eMMC because of this.
>
> Yours,
> Linus Walleij
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH v4] mmc: sdhci-msm: Add pm_runtime and system PM support
From: Ritesh Harjani @ 2016-10-20 14:50 UTC (permalink / raw)
To: Pramod Gurav, ulf.hansson, adrian.hunter, linux-mmc
Cc: linux-kernel, linux-arm-msm, linux-pm
In-Reply-To: <20161018101624.27946-1-pramod.gurav@linaro.org>
Hi Pramod,
Thanks for this patch. Few minor comments.
I have tested your patch on db410c and 8996 based internal platform and
it works fine.
On 10/18/2016 3:46 PM, Pramod Gurav wrote:
> Provides runtime PM callbacks to enable and disable clock resources
> when idle. Also support system PM callbacks to be called during system
> suspend and resume.
>
> Signed-off-by: Pramod Gurav <pramod.gurav@linaro.org>
> ---
>
> Tested on DB410C.
>
> Changes in v4:
> - Remove calls to sdhci_runtime_resume_host/sdhci_runtime_suspend_host
> from runtime callbacks as sdhc msm controller is capable of restoring
> it's register values after clocks are disabled and re-enabled.
>
> Changes in v3:
> - Added CONFIG_PM around runtime pm function.
> - Replaced msm suspend/resume with generic function directly
> - Use SET_SYSTEM_SLEEP_PM_OPS instead of late version
>
> Changes in v2:
> - Moved pm_rutime enabling before adding host
> - Handled pm_rutime in remove
> - Changed runtime handling with reference from sdhci-of-at91.c
>
> drivers/mmc/host/sdhci-msm.c | 66 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 65 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 8ef44a2a..33ec809 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -18,6 +18,7 @@
> #include <linux/of_device.h>
> #include <linux/delay.h>
> #include <linux/mmc/mmc.h>
> +#include <linux/pm_runtime.h>
> #include <linux/slab.h>
>
> #include "sdhci-pltfm.h"
> @@ -658,12 +659,26 @@ static int sdhci_msm_probe(struct platform_device *pdev)
> goto clk_disable;
> }
>
> + pm_runtime_get_noresume(&pdev->dev);
> + pm_runtime_set_active(&pdev->dev);
> + pm_runtime_enable(&pdev->dev);
> + pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
Use a macro MSM_MMC_AUTOSUSPEND_DELAY_MS instead of using 50 directly here.
> + pm_runtime_use_autosuspend(&pdev->dev);
> +
> ret = sdhci_add_host(host);
> if (ret)
> - goto clk_disable;
> + goto pm_runtime_disable;
> +
> + platform_set_drvdata(pdev, host);
No need to set platform_set_drvdata here. sdhci_pltfm_init will do it
anyways.
> +
pm_runtime_mark_last_busy(&pdev->dev) can be added here.
> + pm_runtime_put_autosuspend(&pdev->dev);
>
> return 0;
>
> +pm_runtime_disable:
> + pm_runtime_disable(&pdev->dev);
> + pm_runtime_set_suspended(&pdev->dev);
> + pm_runtime_put_noidle(&pdev->dev);
> clk_disable:
> clk_disable_unprepare(msm_host->clk);
> pclk_disable:
> @@ -685,6 +700,11 @@ static int sdhci_msm_remove(struct platform_device *pdev)
> 0xffffffff);
>
> sdhci_remove_host(host, dead);
> +
> + pm_runtime_get_sync(&pdev->dev);
> + pm_runtime_disable(&pdev->dev);
> + pm_runtime_put_noidle(&pdev->dev);
> +
> clk_disable_unprepare(msm_host->clk);
> clk_disable_unprepare(msm_host->pclk);
> if (!IS_ERR(msm_host->bus_clk))
> @@ -693,12 +713,56 @@ static int sdhci_msm_remove(struct platform_device *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_PM
> +static int sdhci_msm_runtime_suspend(struct device *dev)
> +{
> + struct sdhci_host *host = dev_get_drvdata(dev);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +
> + clk_disable_unprepare(msm_host->clk);
> + clk_disable_unprepare(msm_host->pclk);
> +
> + return 0;
> +}
> +
> +static int sdhci_msm_runtime_resume(struct device *dev)
> +{
> + struct sdhci_host *host = dev_get_drvdata(dev);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> + int ret;
> +
> + ret = clk_prepare_enable(msm_host->clk);
> + if (ret) {
> + dev_err(dev, "clk_enable failed: %d\n", ret);
dev_err(dev, "clk enable failed for core_clk: %d\n", ret);
> + return ret;
> + }
> + ret = clk_prepare_enable(msm_host->pclk);
> + if (ret) {
> + dev_err(dev, "clk_enable failed: %d\n", ret);
dev_err(dev, "clk enable failed for iface_clk: %d\n", ret);
> + clk_disable_unprepare(msm_host->clk);
> + return ret;
> + }
> +
> + return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops sdhci_msm_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> + SET_RUNTIME_PM_OPS(sdhci_msm_runtime_suspend, sdhci_msm_runtime_resume,
> + NULL)
> +};
> +
> static struct platform_driver sdhci_msm_driver = {
> .probe = sdhci_msm_probe,
> .remove = sdhci_msm_remove,
> .driver = {
> .name = "sdhci_msm",
> .of_match_table = sdhci_msm_dt_match,
> + .pm = &sdhci_msm_pm_ops,
> },
> };
>
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* RE: [PATCH v3 2/3] mmc: tmio-mmc: add support for 32bit data port
From: Chris Brandt @ 2016-10-20 14:35 UTC (permalink / raw)
To: Wolfram Sang
Cc: Ulf Hansson, Wolfram Sang, Sergei Shtylyov, Geert Uytterhoeven,
Simon Horman, linux-mmc@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
In-Reply-To: <20161020132852.GB4177@katana>
On 10/20/2016, Wolfram Sang wrote:
> Sadly, I don't have SDHI documentation for this SoC.
In the current hardware manual on renesas.com, the SHDI section is not included.
However, for version 3.0 that will be posted shortly, it will start to include the SDHI section.
> * Does it have a version register (CTL_VERSION)? If so, what does it
> say?
0x820B
> * Does it have SD_BUF0 width setting (named either EXT_ACC or HOST_MODE,
> so far always comes after the version register)? If so, what is its
> layout?
There is a "EXT_SWAP" register. It is the only thing after VERSION.
VERSION = 0xE804E0E2
EXT_SWAP = 0xE804E0F0
Here are the bits in that register:
DMASE (bit 8)
DMA Transfer Size Select
Selects the transfer unit for SD_BUF read/write DMA transfer. Set this bit
in combination with the transfer size set in the DMA Channel
Configuration register.
0: 4-byte (longword) unit
1: 64-byte (longword 16) unit
SDBRSWAP (bit 7)
SD_BUF0 Swap Read*1
When reading from SD_BUF0, data stored in SD_BUF0 can be replaced
and then read.*2
0: The current data is read without replacement.
1: Data replacement for reading proceeds in bytes
SDBWSWAP (bit 6)
SD_BUF0 Swap Write*1
When writing to SD_BUF0, data to be written can be replaced and then
stored in SD_BUF0.*2
0: The current data is written without replacement.
1: Data replacement for writing proceeds in bytes.
> * Does it support DMA?
Yes.
> Is it compatible to the current implementation?
I don't know yet. The current BSP is on 3.14 and it uses DMA, but I'm not sure what's going to happen as I start moving things to 4.9+.
> > + /* if count was multiple of 4 */
> > + if (!(count & 0x3))
> > + return;
> > + buf8 = (u8 *)(buf + (count >> 2));
> > + count %= 4;
>
> To skip the same operation done on 'count' twice, maybe?
>
> buf8 = (u8 *)(buf + (count >> 2));
> count &= 3;
>
> if (!count)
> return;
That looks like it should work.
I can try it out and see if there is something we are missing.
Thanks
Chris
^ permalink raw reply
* Re: [PATCH v3 2/3] mmc: tmio-mmc: add support for 32bit data port
From: Wolfram Sang @ 2016-10-20 13:28 UTC (permalink / raw)
To: Chris Brandt
Cc: Ulf Hansson, Wolfram Sang, Sergei Shtylyov, Geert Uytterhoeven,
Simon Horman, linux-mmc, linux-renesas-soc
In-Reply-To: <20160912141507.6837-3-chris.brandt@renesas.com>
[-- Attachment #1: Type: text/plain, Size: 1367 bytes --]
On Mon, Sep 12, 2016 at 10:15:06AM -0400, Chris Brandt wrote:
> For the r7s72100 SOC, the DATA_PORT register was changed to 32-bits wide.
> Therefore a new flag has been created that will allow 32-bit reads/writes
> to the DATA_PORT register instead of 16-bit (because 16-bits accesses are
> not supported).
>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
> v3:
> * changed loops to memcpy
> v2:
> * changed 'data * 0xFF' to 'data & 0xFF'
> * added 'const' for sd_ctrl_write32_rep
Sadly, I don't have SDHI documentation for this SoC.
* Does it have a version register (CTL_VERSION)? If so, what does it
say?
* Does it have SD_BUF0 width setting (named either EXT_ACC or HOST_MODE,
so far always comes after the version register)? If so, what is its
layout?
Unrelated to this patch but nice to know:
* Does it support DMA? Is it compatible to the current implementation?
That being asked, R-Car SoCs can have SDBUF0 32-bit wide as well (Gen3
even 64-bit). So far, this is only used for DMA, though.
Thanks,
Wolfram
> + /* if count was multiple of 4 */
> + if (!(count & 0x3))
> + return;
> + buf8 = (u8 *)(buf + (count >> 2));
> + count %= 4;
To skip the same operation done on 'count' twice, maybe?
buf8 = (u8 *)(buf + (count >> 2));
count &= 3;
if (!count)
return;
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v4] mmc: sdhci-msm: Add pm_runtime and system PM support
From: Georgi Djakov @ 2016-10-20 13:08 UTC (permalink / raw)
To: Pramod Gurav, ulf.hansson, adrian.hunter, linux-mmc
Cc: linux-kernel, linux-arm-msm, linux-pm, riteshh
In-Reply-To: <20161018101624.27946-1-pramod.gurav@linaro.org>
Hi Pramod,
Thanks for the patch!
On 10/18/2016 01:16 PM, Pramod Gurav wrote:
> Provides runtime PM callbacks to enable and disable clock resources
> when idle. Also support system PM callbacks to be called during system
> suspend and resume.
>
> Signed-off-by: Pramod Gurav <pramod.gurav@linaro.org>
> ---
>
> Tested on DB410C.
>
[..]
> +static int sdhci_msm_runtime_resume(struct device *dev)
> +{
> + struct sdhci_host *host = dev_get_drvdata(dev);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> + int ret;
> +
> + ret = clk_prepare_enable(msm_host->clk);
> + if (ret) {
> + dev_err(dev, "clk_enable failed: %d\n", ret);
> + return ret;
> + }
> + ret = clk_prepare_enable(msm_host->pclk);
> + if (ret) {
> + dev_err(dev, "clk_enable failed: %d\n", ret);
Nit: Maybe mention in the prints which clock failed - core or peripheral.
> + clk_disable_unprepare(msm_host->clk);
> + return ret;
> + }
> +
> + return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops sdhci_msm_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
Nit: Please align with the parenthesis.
> + SET_RUNTIME_PM_OPS(sdhci_msm_runtime_suspend, sdhci_msm_runtime_resume,
> + NULL)
Ditto.
Reviewed-by: Georgi Djakov <georgi.djakov@linaro.org>
BR,
Georgi
^ permalink raw reply
* Re: [PATCH v3 1/3] mmc: sh_mobile_sdhi: add ocr_mask option
From: Wolfram Sang @ 2016-10-20 13:05 UTC (permalink / raw)
To: Chris Brandt
Cc: Ulf Hansson, Wolfram Sang, Sergei Shtylyov, Geert Uytterhoeven,
Simon Horman, linux-mmc, linux-renesas-soc
In-Reply-To: <20160912141507.6837-2-chris.brandt@renesas.com>
[-- Attachment #1: Type: text/plain, Size: 629 bytes --]
On Mon, Sep 12, 2016 at 10:15:05AM -0400, Chris Brandt wrote:
> In moving platforms from board files to DT, there still needs to be a way
> to set the ocr_mask setting for the tmio driver during probe. Without this
> setting, the probe will fail because the supported voltages are not known.
>
> This patch will also traditional platform registration platforms to
> migrate to DT.
>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
I'm fine with this change:
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Maybe some details from this discussion could be added to the commit
message?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: Regression after "do not use CMD13 to get status after speed mode switch"
From: Ulf Hansson @ 2016-10-20 11:00 UTC (permalink / raw)
To: Adrian Hunter
Cc: Chaotian Jing, Linus Walleij, linux-mmc@vger.kernel.org,
linux-arm-msm@vger.kernel.org, Bjorn Andersson, Stephen Boyd,
Andy Gross
In-Reply-To: <6335fe87-9b52-4334-d045-be4b6702bcea@intel.com>
On 20 October 2016 at 09:19, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 20/10/16 10:06, Ulf Hansson wrote:
>> On 20 October 2016 at 04:22, Chaotian Jing <chaotian.jing@mediatek.com> wrote:
>>> On Wed, 2016-10-19 at 18:41 +0200, Ulf Hansson wrote:
>>>> Adrian, Linus,
>>>>
>>>> Thanks for looking into this and reporting!
>>>>
>>>> On 18 October 2016 at 15:23, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>> On 18/10/16 11:36, Linus Walleij wrote:
>>>>>> On Mon, Oct 17, 2016 at 4:32 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>>>>>>
>>>>>>> Before this patch the eMMC is detected and all partitions enumerated
>>>>>>> immediately, but after the patch it doesn't come up at all, except
>>>>>>> sometimes, when it appears minutes (!) after boot, all of a sudden.
>>>>>>
>>>>>> FYI this is what it looks like when it eventually happens:
>>>>>> root@msm8660:/ [ 627.710175] mmc0: new high speed MMC card at address 0001
>>>>>> [ 627.711641] mmcblk0: mmc0:0001 SEM04G 3.69 GiB
>>>>>> [ 627.715485] mmcblk0boot0: mmc0:0001 SEM04G partition 1 1.00 MiB
>>>>>> [ 627.736654] mmcblk0boot1: mmc0:0001 SEM04G partition 2 1.00 MiB
>>>>>> [ 627.747397] mmcblk0rpmb: mmc0:0001 SEM04G partition 3 128 KiB
>>>>>> [ 627.756326] mmcblk0: p1 p2 p3 p4 < p5 p6 p7 p8 p9 p10 p11 p12 p13
>>>>>> p14 p15 p16 p17 p18 p19 p20 p21 >
>>>>>>
>>>>>> So after 627 seconds, a bit hard for users to wait this long for their
>>>>>> root filesystem.
>>>>>
>>>>> If the driver does not support busy detection and the eMMC card provides
>>>>> zero as the cmd6 generic timeout (which it may especially as cmd6 generic
>>>>> timeout wasn't added until eMMCv4.5), then __mmc_switch() defaults to
>>>>> waiting 10 minutes i.e.
>>>>>
>>>>> #define MMC_OPS_TIMEOUT_MS (10 * 60 * 1000) /* 10 minute timeout */
>>>>
>>>> Urgh! Yes, I have verified that this is exactly what happens.
>>>>
>>>>>
>>>>> So removal of CMD13 polling for HS mode (as per commit
>>>>> 08573eaf1a70104f83fdbee9b84e5be03480e9ed) is going to be a problem for some
>>>>> combinations of eMMC cards and host drivers.
>>>>
>>>> I was looking in the __mmc_switch() function, it's just a pain to walk
>>>> trough it :-) So first out I decided to clean it up and factor out the
>>>> polling parts. I will post the patches first out tomorrow morning,
>>>> running some final test right now.
>>>>
>>>> Although, that of course doesn't solve our problem. As I see it we
>>>> only have a few options here.
>>>>
>>>> 1) In case when cmd6 generic timeout isn't available, let's assign
>>>> another empirically selected value.
>>>> 2) Use a specific timeout when switching to HS mode.
>>>> 3) Even if we deploy 1 (and 2), perhaps we still should allow polling
>>>> with CMD13 for switching to HS mode - unless it causes issues for some
>>>> cards/drivers combination?
>>>>
>>>> BTW, I already tried 2) and it indeed solves the problem, although
>>>> depending on the selected timeout, it might delay the card detection
>>>> to process.
>>>>
>>>> Thoughts?
>>>
>>> I just have a try of switching to HS mode with Hynix EMMC, the first
>>> CMD13 gets response of 0x900, but the EMMC is still pull-low DAT0. so
>>> that CMD13 cannot indicate current card status in this case.
>>
>> Thanks for sharing that. Okay, so clearly we have some cards that
>> don't supports polling with CMD13 when switching to HS mode.
>> One could of course add quirks for these kind of cards and do a fixed
>> delay for them, but then to find out which these cards are is going to
>> be hard.
>>
>> It seems like we are left with using a fixed delay. Any ideas of what
>> such delay should be? And should we have one specific for switch to
>> the various speed modes and a different one that overrides the CMD6
>> generic timout, when it doesn't exist?
>
> We have supported polling for a long time, so presumably the broken cards
> are newer ones. What about using busy detection if that is available,
Using HW busy detection, already takes highest precedence.
Although we can improve the behaviour a bit. When
MMC_CAP_WAIT_WHILE_BUSY isn't available but when ->card_busy() is, as
in a submitted patch [1].
> otherwise using polling if the card does not support HS200, otherwise using
Maybe we should first investigate the behaviour for some old cards!?
Perhaps it's more or less by "accident" that the cards has been
working and maybe that's because the delay needed is really small?
> the cmd6 generic timeout unless it is zero, and finally falling back to an
> arbitrary waiting time.
>
If I understand correct, you suggest we move back to allow polling for
older cards, but only when switching to high-speed mode, right?
Kind regards
Uffe
[1]
https://patchwork.kernel.org/patch/9386229/
^ permalink raw reply
* [GIT PULL] MMC fixes for v.4.9 rc2
From: Ulf Hansson @ 2016-10-20 10:33 UTC (permalink / raw)
To: Linus, linux-mmc, linux-kernel@vger.kernel.org
Cc: Alan Stern, Ritesh Raj Sarraf, Adrian Hunter, Jaehoon Chung
Hi Linus,
Here are some mmc fixes intended for v4.9 rc2. They are based on a
commit during the merge window and has been tested a while on top of
v4.9-rc1, via my next branch.
This time I have also included a few changes for a memstick driver
which has a corresponding mmc driver. They use the same USB device as
parent, hence both needs to play nice with runtime PM, which they
didn't.
Unfortunate, the memstick driver didn't have a maintainer, so I
couldn't receive an ack of the changes for it.
More details can as usual be found in the signed tag. Please pull this in!
Kind regards
Ulf Hansson
The following changes since commit 24532f768121b07b16178ffb40442ece43365cbd:
Merge branch 'for-4.9/block-smp' of git://git.kernel.dk/linux-block
(2016-10-09 17:32:20 -0700)
are available in the git repository at:
http://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git tags/mmc-v4.9-rc1
for you to fetch changes up to 82bb095ee9dcd9380b6ab608b5fa9f64e65f3a3b:
MAINTAINERS: mmc: Move the mmc tree to kernel.org (2016-10-17 15:46:05 +0200)
----------------------------------------------------------------
MMC core:
- Update MAINTAINERS as the mmc tree moved to kernel.org
- A few fixes for HS400es mode
- A few other minor fixes
MMC host:
- sdhci: Fix an issue when dealing with stop commands
- sdhci-pci: Fix a bus power failure issue
- sdhci-esdhc-imx: Correct two register accesses
- sdhci-of-arasan: Fix the 1.8V I/O signal switch behaviour
- rtsx_usb_sdmmc: Fix runtime PM issues
Other: (Because of no maintainer)
- memstick: rtsx_usb_ms: Fix runtime PM issues
----------------------------------------------------------------
Aaron Brice (1):
mmc: sdhci-esdhc-imx: Correct two register accesses
Adrian Hunter (4):
mmc: sdhci: Fix SDHCI_QUIRK2_STOP_WITH_TC
mmc: sdhci: Rename sdhci_set_power() to sdhci_set_power_noreg()
mmc: sdhci-pci: Let devices define their own sdhci_ops
mmc: sdhci-pci: Fix bus power failing to enable for some Intel controllers
Alan Stern (1):
memstick: rtsx_usb_ms: Runtime resume the device when polling for cards
Baoyou Xie (1):
mmc: block: add missing header dependencies
Haibo Chen (1):
mmc: sdhci: cast unsigned int to unsigned long long to avoid
unexpeted error
Jiri Slaby (1):
mmc: core: Annotate cmd_hdr as __le32
Shawn Lin (3):
mmc: core: switch to 1V8 or 1V2 for hs400es mode
mmc: core: changes frequency to hs_max_dtr when selecting hs400es
mmc: sdhci-of-arasan: add sdhci_arasan_voltage_switch for arasan, 5.1
Ulf Hansson (4):
mmc: rtsx_usb_sdmmc: Avoid keeping the device runtime resumed when unused
mmc: rtsx_usb_sdmmc: Handle runtime PM while changing the led
memstick: rtsx_usb_ms: Manage runtime PM when accessing the device
MAINTAINERS: mmc: Move the mmc tree to kernel.org
Wei Yongjun (1):
mmc: sdhci-of-arasan: Fix non static symbol warning
MAINTAINERS | 2 +-
drivers/memstick/host/rtsx_usb_ms.c | 6 +++++
drivers/mmc/card/block.c | 3 ++-
drivers/mmc/card/queue.h | 2 +-
drivers/mmc/core/mmc.c | 12 +++++++++
drivers/mmc/host/rtsx_usb_sdmmc.c | 7 ++---
drivers/mmc/host/sdhci-esdhc-imx.c | 23 +++++++++++++++-
drivers/mmc/host/sdhci-of-arasan.c | 26 +++++++++++++++++-
drivers/mmc/host/sdhci-pci-core.c | 54 ++++++++++++++++++++++++++++++++++++-
drivers/mmc/host/sdhci-pci.h | 2 ++
drivers/mmc/host/sdhci-pxav3.c | 2 +-
drivers/mmc/host/sdhci.c | 42 ++++++++++++++---------------
drivers/mmc/host/sdhci.h | 2 ++
13 files changed, 149 insertions(+), 34 deletions(-)
^ permalink raw reply
* RE: [PATCH] mmc: sdhci-esdhc-imx: make sure usdhc clock enabled while doing suspend
From: Bough Chen @ 2016-10-20 10:21 UTC (permalink / raw)
To: Dong Aisheng, Ulf Hansson; +Cc: Adrian Hunter, linux-mmc, A.S. Dong
In-Reply-To: <CAA+hA=SOk4K6QrQ4T_FCTpuZs1tKfpTA=wHnhvFX7A=30qsLsA@mail.gmail.com>
> -----Original Message-----
> From: Dong Aisheng [mailto:dongas86@gmail.com]
> Sent: Wednesday, October 19, 2016 5:18 PM
> To: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Bough Chen <haibo.chen@nxp.com>; Adrian Hunter
> <adrian.hunter@intel.com>; linux-mmc <linux-mmc@vger.kernel.org>; A.S.
> Dong <aisheng.dong@nxp.com>
> Subject: Re: [PATCH] mmc: sdhci-esdhc-imx: make sure usdhc clock enabled
> while doing suspend
>
> Hi Ulf,
>
> On Tue, Oct 18, 2016 at 5:18 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > On 18 October 2016 at 09:39, Haibo Chen <haibo.chen@nxp.com> wrote:
> >> When suspend usdhc, it will access usdhc register. So usdhc clock
> >> should be enabled, otherwise the access usdhc register will return
> >> error or cause system hung.
> >>
> >> Take this into consideration, if system enable a usdhc and do not
> >> connect any SD/SDIO/MMC card, after system boot up, this usdhc will
> >> do runtime suspend, and close all usdhc clock. At this time, if
> >> suspend the system, due to no card persent, usdhc runtime resume will
> >> not be called. So usdhc clock still closed, then in suspend, once
> >> access usdhc register, system hung or bus error return.
> >>
> >> This patch make sure usdhc clock always enabled while doing usdhc
> >> suspend.
> >
> > Yes, and since the clocks are kept enabled during system suspend that
> > means wasting power, doesn't it!?
> >
>
> IMX SoCs will disable all modules clocks in system stop mode automatically by
> hardware even it's enabled before CCGR value Clock Activity Description:
> 00 clock is off during all modes. stop enter hardware handshake is disabled.
> 01 clock is on in run mode, but off in wait and stop modes
> 10 Not applicable (Reserved).
> 11 clock is on during all modes, except stop mode.
>
> Although HW will gate off it automatically, but i think it's still good to align the
> state between SW and HW.
>
Agree
> > May I propose another solution. Currently you deal only with clock
> > gating/ungating during runtime suspend/resume. I am wondering whether
> > you could extend those operations to be similar to what is needed
> > during system suspend/resume?
> >
>
> IMX driver are calling sdhci_runtime_suspend_host() and
> sdhci_suspend_host() for runtime
> suspend and system sleep case respectively.
> Those two APIs definitions are different.
> e.g. sdhci_suspend_host will disable card detection and enable wakeup if any
> while
> sdhci_runtime_suspend_host() not.
>
> It may not be suitable to extend runtime operations to be similar as sleep pm
> operations if using common sdhci suspend function, unless we implement
> totoally IMX specific PM/Runtime PM function.
>
>
> Another option may be like what omap_hsmmc does:
>
> Something like:
>
> int sdhci_esdhc_suspend(struct device *dev) {
> pm_runtime_get_sync(host->dev);
> ret = sdhci_pltfm_suspend(dev);
> pm_runtime_put_sync(host->dev);
>
> return ret;
> }
>
I think this method still has the same issue.
According to the /Documentation/power/runtime_pm.txt
370 int pm_runtime_get_sync(struct device *dev);
371 - increment the device's usage counter, run pm_runtime_resume(dev) and
372 return its result
385 int pm_runtime_put_sync(struct device *dev);
386 - decrement the device's usage counter; if the result is 0 then run
387 pm_runtime_idle(dev) and return its result
pm_runtime_put_sync() will not call pm_runtime_suspend(), so clock still enabled, the same issue.
> int sdhci_esdhc_resume(struct device *dev) {
> pm_runtime_get_sync(host->dev);
>
> ...
> ret = sdhci_pltfm_resume(dev);
>
> pm_runtime_mark_last_busy(host->dev);
> pm_runtime_put_autosuspend(host->dev);
>
> return ret;
> }
>
> Does that seem ok?
>
> > If that is possible, you can instead deploy the runtime PM centric
> > approach and get system suspend/resume for free. All you would have to
> > do is to assign the system PM callbacks to
> > pm_runtime_force_suspend|resume(). In that way, the above problem
> > would be solved and you don't need to keep the clocks enabled during
> > system suspend/resume.
> >
> > Kind regards
> > Uffe
> >
>
> Regards
> Dong Aisheng
>
> >>
> >> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> >> ---
> >> drivers/mmc/host/sdhci-esdhc-imx.c | 13 ++++++++++++-
> >> 1 file changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> >> b/drivers/mmc/host/sdhci-esdhc-imx.c
> >> index 7123ef9..1df3846 100644
> >> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> >> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> >> @@ -1322,17 +1322,28 @@ static int sdhci_esdhc_suspend(struct device
> >> *dev) {
> >> struct sdhci_host *host = dev_get_drvdata(dev);
> >>
> >> +#ifdef CONFIG_PM
> >> + pm_runtime_get_sync(host->mmc->parent);
> >> +#endif
> >> +
> >> return sdhci_suspend_host(host); }
> >>
> >> static int sdhci_esdhc_resume(struct device *dev) {
> >> struct sdhci_host *host = dev_get_drvdata(dev);
> >> + int ret;
> >>
> >> /* re-initialize hw state in case it's lost in low power mode */
> >> sdhci_esdhc_imx_hwinit(host);
> >> + ret = sdhci_resume_host(host);
> >>
> >> - return sdhci_resume_host(host);
> >> +#ifdef CONFIG_PM
> >> + pm_runtime_mark_last_busy(host->mmc->parent);
> >> + pm_runtime_put_autosuspend(host->mmc->parent);
> >> +#endif
> >> +
> >> + return ret;
> >> }
> >> #endif
> >>
> >> --
> >> 1.9.1
> >>
^ permalink raw reply
* Re: [PATCH v7 09/11] arm64: dts: r8a7796: salvator-x: enable SDHI0 & 3
From: Simon Horman @ 2016-10-20 9:19 UTC (permalink / raw)
To: Wolfram Sang
Cc: Wolfram Sang, Ulf Hansson, Magnus Damm, linux-mmc,
linux-renesas-soc, linux-arm-kernel
In-Reply-To: <20160916103218.GM1411@katana>
On Fri, Sep 16, 2016 at 12:32:18PM +0200, Wolfram Sang wrote:
> On Tue, Sep 13, 2016 at 12:57:06PM +0200, Simon Horman wrote:
> > Enable the exposed SD card slots in the DT of the r8a7796/salvator-x.
> >
> > Based on work for the r8a7795/salvator-x by Ai Kyuse.
> >
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Thanks, I have queued this up.
^ permalink raw reply
* Re: [PATCH v7 08/11] arm64: dts: r8a7796: add SDHI nodes
From: Simon Horman @ 2016-10-20 9:19 UTC (permalink / raw)
To: Wolfram Sang
Cc: Wolfram Sang, Ulf Hansson, Magnus Damm, linux-mmc,
linux-renesas-soc, linux-arm-kernel
In-Reply-To: <20160916105321.GB7566@verge.net.au>
On Fri, Sep 16, 2016 at 12:53:22PM +0200, Simon Horman wrote:
> On Fri, Sep 16, 2016 at 12:30:48PM +0200, Wolfram Sang wrote:
> > On Tue, Sep 13, 2016 at 12:57:05PM +0200, Simon Horman wrote:
> > > Add SDHI nodes to the DT of the r8a7796 SoC.
> > >
> > > Based on the DT of the r8a7795 SoC.
> > >
> > > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > If you remove the two lines you removed from H3 already:
>
> Thanks, done.
>
> > Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
I have queued this up for v4.10.
^ permalink raw reply
* Re: [PATCH v7 10/11] arm64: dts: r8a7796: salvator-x: enable UHS for SDHI 0 & 3
From: Simon Horman @ 2016-10-20 9:19 UTC (permalink / raw)
To: Wolfram Sang
Cc: Wolfram Sang, Ulf Hansson, Magnus Damm, linux-mmc,
linux-renesas-soc, linux-arm-kernel
In-Reply-To: <20160916103317.GN1411@katana>
On Fri, Sep 16, 2016 at 12:33:17PM +0200, Wolfram Sang wrote:
> On Tue, Sep 13, 2016 at 12:57:07PM +0200, Simon Horman wrote:
> > Based on work for the r8a7796 by Wolfram Sang.
> >
> > Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
>
> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Thanks, I have queued this up.
^ permalink raw reply
* [PATCH 4/4] mmc: core: Don't use ->card_busy() and CMD13 in combination when polling
From: Ulf Hansson @ 2016-10-20 8:19 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson
Cc: Jaehoon Chung, Adrian Hunter, Linus Walleij, Chaotian Jing
In-Reply-To: <1476951579-26125-1-git-send-email-ulf.hansson@linaro.org>
When polling for busy after sending a MMC_SWITCH command, both the optional
->card_busy() callback and CMD13 are being used in conjunction.
This doesn't make sense. Instead it's more reasonable to rely solely on the
->card_busy() callback when it exists. Let's change that and instead use
the CMD13 as a fall-back. In this way we avoid sending CMD13, unless it's
really needed.
Within this context, let's also take the opportunity to make some
additional clean-ups and clarifications to the related code.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/mmc_ops.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index a84a880..481bbdb 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -495,34 +495,32 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
timeout = jiffies + msecs_to_jiffies(timeout_ms) + 1;
do {
/*
- * Due to the possibility of being preempted after
- * sending the status command, check the expiration
- * time first.
+ * Due to the possibility of being preempted while polling,
+ * check the expiration time first.
*/
expired = time_after(jiffies, timeout);
- if (send_status) {
+
+ if (host->ops->card_busy) {
+ busy = host->ops->card_busy(host);
+ } else {
err = __mmc_send_status(card, &status, ignore_crc);
if (err)
return err;
- }
- if (host->ops->card_busy) {
- if (!host->ops->card_busy(host))
- break;
- busy = true;
+ busy = R1_CURRENT_STATE(status) == R1_STATE_PRG;
}
- /* Timeout if the device never leaves the program state. */
- if (expired &&
- (R1_CURRENT_STATE(status) == R1_STATE_PRG || busy)) {
- pr_err("%s: Card stuck in programming state! %s\n",
+ /* Timeout if the device still remains busy. */
+ if (expired && busy) {
+ pr_err("%s: Card stuck being busy! %s\n",
mmc_hostname(host), __func__);
return -ETIMEDOUT;
}
- } while (R1_CURRENT_STATE(status) == R1_STATE_PRG || busy);
+ } while (busy);
- err = mmc_switch_status_error(host, status);
+ if (host->ops->card_busy && send_status)
+ return mmc_switch_status(card);
- return err;
+ return mmc_switch_status_error(host, status);
}
/**
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox