Re: [PATCH V2 0/4] mmc: sdhci: Disable re-tuning for HS400
On 12/06/2014 01:25 AM, Adrian Hunter wrote: Hi Here is V2 of patches to disable re-tuning for HS400. As described in patch 4, re-tuning for HS400 has to be done in HS200 mode, but there is no support for that, so re-tuning needs to be disabled until support is added. Changes in V2: Added mmc: sdhci: Simplify use of tuning timer patch. Changed mmc: sdhci: Disable re-tuning for HS400 so that the tuning timer is not started instead of stopping it when HS400 mode is selected. Adrian Hunter (4): mmc: sdhci: Tuning should not change max_blk_count mmc: sdhci: Add out_unlock to sdhci_execute_tuning mmc: sdhci: Simplify use of tuning timer mmc: sdhci: Disable re-tuning for HS400 For patch 1-3: Reviewed-by: Aaron Lu aaron...@intel.com Thanks, Aaron drivers/mmc/host/sdhci.c | 66 ++- include/linux/mmc/sdhci.h | 1 + 2 files changed, 43 insertions(+), 24 deletions(-) Regards Adrian -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 07/11] mmc: sdio: Convert to dev_pm_domain_attach|detach()
On 10/02/2014 08:27 AM, Dmitry Torokhov wrote:
Hi Ulf,
On Fri, Sep 19, 2014 at 08:27:40PM +0200, Ulf Hansson wrote:
Previously only the ACPI PM domain was supported by the sdio bus.
Let's convert to the common attach/detach functions for PM domains,
which currently means we are extending the support to include the
generic PM domain as well.
Cc: linux-mmc@vger.kernel.org
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
Reviewed-by: Kevin Hilman khil...@linaro.org
---
drivers/mmc/core/sdio_bus.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index 4fa8fef9..1df0fc6 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -315,7 +315,7 @@ int sdio_add_func(struct sdio_func *func)
ret = device_add(func-dev);
if (ret == 0) {
sdio_func_set_present(func);
-acpi_dev_pm_attach(func-dev, false);
+dev_pm_domain_attach(func-dev, false);
Admittedly it is not brought in by your change, but I am a bit worried
about this code. In all other busses we attach power domain to a device
before probing it (and detach if probe fails). Why here we only attach
it to power domain after adding the device to the bus? If driver for the
function has already been loaded the probe() would run without device
being attached to a power domain...
Sorry for replying late...
I see your concern, but it should be safe here. The dev_pm_domain_attach
does primarily two things(for ACPI based system): assign the pm_domian
field so that later system and runtime PM transitions we have proper
callbacks for this device; power on the device if needed.
I was using Broadcom BCM43241 SDIO wifi card when developing the code,
it doesn't need the pm_domain set in its probe function and since the
SDIO wifi card is powered on after system boot, no power on is needed
either. That's why I added the attach function after device_add to save
a little code(detach if probe fails).
Feel free to change the sequence if the current one is a bad one, I'm OK
with either case.
Adding Aaron as he's the author of the original change.
Thanks,
Aaron
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: one doubt about mmc_sdio_init_card function
On 07/01/2014 01:39 PM, Fu, Zhonghui wrote:
Hi, all
The mmc_sdio_init_card(drivers/mmc/core/sdio.c) function calls
mmc_alloc_card(drivers/mmc/core/bus.c) function to allocate a card structure.
card-dev.bus is assigned with mmc_bus_type in mmc_alloc_card function. Why
not assign sdio_bus_type to card-dev.bus?
sdio card, mmc card, sd card are all devices on the mmc bus, hence their
bus type is set to mmc_bus_type.
sdio function device is a device on the sdio bus, hence its bus type is
sdio_bus_type.
Hope this helps,
Aaron
struct mmc_card *mmc_alloc_card(struct mmc_host *host, struct device_type
*type)
{
struct mmc_card *card;
card = kzalloc(sizeof(struct mmc_card), GFP_KERNEL);
if (!card)
return ERR_PTR(-ENOMEM);
card-host = host;
device_initialize(card-dev);
card-dev.parent = mmc_classdev(host);
card-dev.bus = mmc_bus_type;
card-dev.release = mmc_release_card;
card-dev.type = type;
return card;
}
Thanks,
Zhonghui
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RESEND:[PATCH V3] sdhci: only reprogram retuning timer when flag is set
On 02/22/2014 03:59 AM, Arend van Spriel wrote:
When the host-tuning_count is zero it means that the
retuning is disabled. This is checked on the first
run of sdhci_execute_tuning() by the if statement below:
if (!(host-flags SDHCI_NEEDS_RETUNING) host-tuning_count
(host-tuning_mode == SDHCI_TUNING_MODE_1)) {
So only when tuning_count is non-zero it will set the host
flag SDHCI_USING_RETUNING_TIMER. The else statement is only
for re-programming the timer, which means that flag must be
set. Because that is not checked the else statement is executed
in the first run when tuning_count is zero.
This was seen on a host controller which indicated
SDHCI_TUNING_MODE_1 (0) and tuning_count being zero. Suspect
that (one of) these registers is not properly set.
Cc: Dong Aisheng donga...@gmail.com
Cc: Aaron Lu aaron@gmail.com
Signed-off-by: Arend van Spriel ar...@broadcom.com
In addition to solve your problem, this patch also makes sense in the
common case, so:
Reviewed-by: Aaron Lu aaron...@intel.com
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] mmc: core: Invoke sdio func driver's PM callbacks from the sdio bus
On 03/12/2014 11:44 AM, Dong, Chuanxiao wrote:
Hi Aaron,
This patch is tested on Intel platform, and SDIO function driver's
suspend/resume callback will only be called once, which fixed this issue.
Previously, they can be called twice.
Here is the tested-by:
Tested-by: xiaoming wang xiaoming.w...@intel.com
Tested-by: Chuanxiao Dong chuanxiao.d...@intel.com
Thanks a lot for the test!
-Aaron
Thanks
Chuanxiao
-Original Message-
From: Lu, Aaron
Sent: Wednesday, March 12, 2014 10:36 AM
To: Ulf Hansson; linux-mmc@vger.kernel.org; Chris Ball; Liu, Chuansheng;
Dong,
Chuanxiao
Cc: linux-ker...@vger.kernel.org; NeilBrown; Rafael J. Wysocki
Subject: Re: [RFC PATCH] mmc: core: Invoke sdio func driver's PM callbacks
from the
sdio bus
Hi Chuansheng Chuanxiao,
Can you please help us testing this patch on your platform and let us know
the test
result? Thanks.
-Aaron
On 02/28/2014 07:49 PM, Ulf Hansson wrote:
The sdio func device is added to the driver model after the card
device.
This means the sdio func device will be suspend before the card device
and thus resumed after. The consequence are the mmc core don't
explicity need to protect itself from receiving sdio requests in
suspended state. Instead that can be handled from the sdio bus, which
is thus invokes the PM callbacks instead of old dummy function.
In the case were the sdio func driver don't implement the PM callbacks
the mmc core will in the early phase of system suspend, remove the
card from the driver model and thus power off it.
Cc: Aaron Lu aaron...@intel.com
Cc: NeilBrown ne...@suse.de
Cc: Rafael J. Wysocki r...@rjwysocki.net
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
Note, this patch has only been compile tested. Would appreciate if
some with SDIO and a sdio func driver could help out to test this.
Especially the libertas driver would be nice.
---
drivers/mmc/core/sdio.c | 45
---
drivers/mmc/core/sdio_bus.c | 14 +-
2 files changed, 5 insertions(+), 54 deletions(-)
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c index
4d721c6..9933e42 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -943,40 +943,21 @@ static int mmc_sdio_pre_suspend(struct mmc_host
*host)
*/
static int mmc_sdio_suspend(struct mmc_host *host) {
- int i, err = 0;
-
- for (i = 0; i host-card-sdio_funcs; i++) {
- struct sdio_func *func = host-card-sdio_func[i];
- if (func sdio_func_present(func) func-dev.driver) {
- const struct dev_pm_ops *pmops = func-dev.driver-pm;
- err = pmops-suspend(func-dev);
- if (err)
- break;
- }
- }
- while (err --i = 0) {
- struct sdio_func *func = host-card-sdio_func[i];
- if (func sdio_func_present(func) func-dev.driver) {
- const struct dev_pm_ops *pmops = func-dev.driver-pm;
- pmops-resume(func-dev);
- }
- }
-
- if (!err mmc_card_keep_power(host) mmc_card_wake_sdio_irq(host))
{
+ if (mmc_card_keep_power(host) mmc_card_wake_sdio_irq(host)) {
mmc_claim_host(host);
sdio_disable_wide(host-card);
mmc_release_host(host);
}
- if (!err !mmc_card_keep_power(host))
+ if (!mmc_card_keep_power(host))
mmc_power_off(host);
- return err;
+ return 0;
}
static int mmc_sdio_resume(struct mmc_host *host) {
- int i, err = 0;
+ int err = 0;
BUG_ON(!host);
BUG_ON(!host-card);
@@ -1019,24 +1000,6 @@ static int mmc_sdio_resume(struct mmc_host *host)
wake_up_process(host-sdio_irq_thread);
mmc_release_host(host);
- /*
-* If the card looked to be the same as before suspending, then
-* we proceed to resume all card functions. If one of them returns
-* an error then we simply return that error to the core and the
-* card will be redetected as new. It is the responsibility of
-* the function driver to perform further tests with the extra
-* knowledge it has of the card to confirm the card is indeed the
-* same as before suspending (same MAC address for network cards,
-* etc.) and return an error otherwise.
-*/
- for (i = 0; !err i host-card-sdio_funcs; i++) {
- struct sdio_func *func = host-card-sdio_func[i];
- if (func sdio_func_present(func) func-dev.driver) {
- const struct dev_pm_ops *pmops = func-dev.driver-pm;
- err = pmops-resume(func-dev);
- }
- }
-
host-pm_flags = ~MMC_PM_KEEP_POWER;
return err;
}
diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index 92d1ba8..4fa8fef9 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -197,20 +197,8 @@ static int
Re: [RFC PATCH] mmc: core: Invoke sdio func driver's PM callbacks from the sdio bus
Hi Chuansheng Chuanxiao,
Can you please help us testing this patch on your platform
and let us know the test result? Thanks.
-Aaron
On 02/28/2014 07:49 PM, Ulf Hansson wrote:
The sdio func device is added to the driver model after the card
device.
This means the sdio func device will be suspend before the card device
and thus resumed after. The consequence are the mmc core don't
explicity need to protect itself from receiving sdio requests in
suspended state. Instead that can be handled from the sdio bus, which
is thus invokes the PM callbacks instead of old dummy function.
In the case were the sdio func driver don't implement the PM callbacks
the mmc core will in the early phase of system suspend, remove the
card from the driver model and thus power off it.
Cc: Aaron Lu aaron...@intel.com
Cc: NeilBrown ne...@suse.de
Cc: Rafael J. Wysocki r...@rjwysocki.net
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
Note, this patch has only been compile tested. Would appreciate if some with
SDIO and a sdio func driver could help out to test this. Especially the
libertas driver would be nice.
---
drivers/mmc/core/sdio.c | 45
---
drivers/mmc/core/sdio_bus.c | 14 +-
2 files changed, 5 insertions(+), 54 deletions(-)
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 4d721c6..9933e42 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -943,40 +943,21 @@ static int mmc_sdio_pre_suspend(struct mmc_host *host)
*/
static int mmc_sdio_suspend(struct mmc_host *host)
{
- int i, err = 0;
-
- for (i = 0; i host-card-sdio_funcs; i++) {
- struct sdio_func *func = host-card-sdio_func[i];
- if (func sdio_func_present(func) func-dev.driver) {
- const struct dev_pm_ops *pmops = func-dev.driver-pm;
- err = pmops-suspend(func-dev);
- if (err)
- break;
- }
- }
- while (err --i = 0) {
- struct sdio_func *func = host-card-sdio_func[i];
- if (func sdio_func_present(func) func-dev.driver) {
- const struct dev_pm_ops *pmops = func-dev.driver-pm;
- pmops-resume(func-dev);
- }
- }
-
- if (!err mmc_card_keep_power(host) mmc_card_wake_sdio_irq(host)) {
+ if (mmc_card_keep_power(host) mmc_card_wake_sdio_irq(host)) {
mmc_claim_host(host);
sdio_disable_wide(host-card);
mmc_release_host(host);
}
- if (!err !mmc_card_keep_power(host))
+ if (!mmc_card_keep_power(host))
mmc_power_off(host);
- return err;
+ return 0;
}
static int mmc_sdio_resume(struct mmc_host *host)
{
- int i, err = 0;
+ int err = 0;
BUG_ON(!host);
BUG_ON(!host-card);
@@ -1019,24 +1000,6 @@ static int mmc_sdio_resume(struct mmc_host *host)
wake_up_process(host-sdio_irq_thread);
mmc_release_host(host);
- /*
- * If the card looked to be the same as before suspending, then
- * we proceed to resume all card functions. If one of them returns
- * an error then we simply return that error to the core and the
- * card will be redetected as new. It is the responsibility of
- * the function driver to perform further tests with the extra
- * knowledge it has of the card to confirm the card is indeed the
- * same as before suspending (same MAC address for network cards,
- * etc.) and return an error otherwise.
- */
- for (i = 0; !err i host-card-sdio_funcs; i++) {
- struct sdio_func *func = host-card-sdio_func[i];
- if (func sdio_func_present(func) func-dev.driver) {
- const struct dev_pm_ops *pmops = func-dev.driver-pm;
- err = pmops-resume(func-dev);
- }
- }
-
host-pm_flags = ~MMC_PM_KEEP_POWER;
return err;
}
diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index 92d1ba8..4fa8fef9 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -197,20 +197,8 @@ static int sdio_bus_remove(struct device *dev)
#ifdef CONFIG_PM
-#ifdef CONFIG_PM_SLEEP
-static int pm_no_operation(struct device *dev)
-{
- /*
- * Prevent the PM core from calling SDIO device drivers' suspend
- * callback routines, which it is not supposed to do, by using this
- * empty function as the bus type suspend callaback for SDIO.
- */
- return 0;
-}
-#endif
-
static const struct dev_pm_ops sdio_bus_pm_ops = {
- SET_SYSTEM_SLEEP_PM_OPS(pm_no_operation, pm_no_operation)
+ SET_SYSTEM_SLEEP_PM_OPS(pm_generic_suspend, pm_generic_resume)
SET_RUNTIME_PM_OPS(
pm_generic_runtime_suspend
Re: SDIO driver return -ENOSYS behaviour change?
On 02/28/2014 04:30 PM, Ulf Hansson wrote: On 28 February 2014 03:37, Aaron Lu aaron...@intel.com wrote: On 02/27/2014 09:05 PM, Ulf Hansson wrote: I think it's more a matter of having a controlled suspend sequence. The mmc core are not able to serve any new SDIO requests while it is suspended, therefore it tells the sdio func driver about when it safe to send request - using it's PM callbacks. Does this mean after the function device is suspended from PM core's pespective, the mmc core will still send some requests to the function device? I did see one such request, disable_width, get sent after the function driver's suspend callback is invoked, don't know if there are others. Nope. The mmc core will send request to the card device during suspend. Those are part's of a sequence to put the card in a proper suspend mode. I suppose this request is sent after calling function driver's suspend callback? BTW, what request is it? From the mmc_sdio_suspend function I can see two things are done: 1 function device driver's suspend callback is called; 2 optionally disable_width and power off the card according to some flags. So once we fixed the -ENOSYS problem, can we have the function device run its own suspend callback and have the mmc_sdio_suspend just did the disable_width and power off thing? That requires that all sdio func devices get suspended before the card devices. (And resumed after, but that will of course be implicit.) Yes, and this is guaranteed by PM core as the function device is the child of the card device. I guess the guarantee for that is already present, since we have added the func device to the driver model after we have added the card device at mmc_sdio_attach(). So, I suppose this could work. Exactly. Do you want me to create a patch that you can test? That would be good. I do not have any SDIO card at hand, but I would be glad to review the patch. Thanks, Aaron -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] mmc: core: Invoke sdio func driver's PM callbacks from the sdio bus
On 02/28/2014 07:49 PM, Ulf Hansson wrote:
The sdio func device is added to the driver model after the card
device.
This means the sdio func device will be suspend before the card device
and thus resumed after. The consequence are the mmc core don't
explicity need to protect itself from receiving sdio requests in
suspended state. Instead that can be handled from the sdio bus, which
is thus invokes the PM callbacks instead of old dummy function.
In the case were the sdio func driver don't implement the PM callbacks
the mmc core will in the early phase of system suspend, remove the
card from the driver model and thus power off it.
Cc: Aaron Lu aaron...@intel.com
Cc: NeilBrown ne...@suse.de
Cc: Rafael J. Wysocki r...@rjwysocki.net
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
Note, this patch has only been compile tested. Would appreciate if some with
SDIO and a sdio func driver could help out to test this. Especially the
libertas driver would be nice.
---
drivers/mmc/core/sdio.c | 45
---
drivers/mmc/core/sdio_bus.c | 14 +-
2 files changed, 5 insertions(+), 54 deletions(-)
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 4d721c6..9933e42 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -943,40 +943,21 @@ static int mmc_sdio_pre_suspend(struct mmc_host *host)
*/
static int mmc_sdio_suspend(struct mmc_host *host)
{
- int i, err = 0;
-
- for (i = 0; i host-card-sdio_funcs; i++) {
- struct sdio_func *func = host-card-sdio_func[i];
- if (func sdio_func_present(func) func-dev.driver) {
- const struct dev_pm_ops *pmops = func-dev.driver-pm;
- err = pmops-suspend(func-dev);
- if (err)
- break;
- }
- }
- while (err --i = 0) {
- struct sdio_func *func = host-card-sdio_func[i];
- if (func sdio_func_present(func) func-dev.driver) {
- const struct dev_pm_ops *pmops = func-dev.driver-pm;
- pmops-resume(func-dev);
- }
- }
-
- if (!err mmc_card_keep_power(host) mmc_card_wake_sdio_irq(host)) {
+ if (mmc_card_keep_power(host) mmc_card_wake_sdio_irq(host)) {
mmc_claim_host(host);
sdio_disable_wide(host-card);
mmc_release_host(host);
}
- if (!err !mmc_card_keep_power(host))
+ if (!mmc_card_keep_power(host))
mmc_power_off(host);
- return err;
+ return 0;
}
static int mmc_sdio_resume(struct mmc_host *host)
{
- int i, err = 0;
+ int err = 0;
BUG_ON(!host);
BUG_ON(!host-card);
@@ -1019,24 +1000,6 @@ static int mmc_sdio_resume(struct mmc_host *host)
wake_up_process(host-sdio_irq_thread);
mmc_release_host(host);
- /*
- * If the card looked to be the same as before suspending, then
- * we proceed to resume all card functions. If one of them returns
- * an error then we simply return that error to the core and the
- * card will be redetected as new. It is the responsibility of
- * the function driver to perform further tests with the extra
- * knowledge it has of the card to confirm the card is indeed the
- * same as before suspending (same MAC address for network cards,
- * etc.) and return an error otherwise.
- */
- for (i = 0; !err i host-card-sdio_funcs; i++) {
- struct sdio_func *func = host-card-sdio_func[i];
- if (func sdio_func_present(func) func-dev.driver) {
- const struct dev_pm_ops *pmops = func-dev.driver-pm;
- err = pmops-resume(func-dev);
- }
- }
-
host-pm_flags = ~MMC_PM_KEEP_POWER;
return err;
}
diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index 92d1ba8..4fa8fef9 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -197,20 +197,8 @@ static int sdio_bus_remove(struct device *dev)
#ifdef CONFIG_PM
-#ifdef CONFIG_PM_SLEEP
-static int pm_no_operation(struct device *dev)
-{
- /*
- * Prevent the PM core from calling SDIO device drivers' suspend
- * callback routines, which it is not supposed to do, by using this
- * empty function as the bus type suspend callaback for SDIO.
- */
- return 0;
-}
-#endif
-
static const struct dev_pm_ops sdio_bus_pm_ops = {
- SET_SYSTEM_SLEEP_PM_OPS(pm_no_operation, pm_no_operation)
+ SET_SYSTEM_SLEEP_PM_OPS(pm_generic_suspend, pm_generic_resume)
The same thing can be achieved by not defining the two callbacks here,
the PM core will use driver's callback in that case, but that is not a
big deal.
Reviewed-by: Aaron Lu aaron...@intel.com
Thanks
SDIO driver return -ENOSYS behaviour change?
Hi Ulf, I was tracking some SDIO suspend problem and came across this. As Neil mentioned here: http://lkml.org/lkml/2012/3/25/20 Quote: SDIO (and possible MMC in general) has a protocol where the suspend method can return -ENOSYS and this means There is no point in suspending, just turn me off. It seems that the following commit: commit 810caddba42a54fe5db4e2664757a9a334ba359c Author: Ulf Hansson ulf.hans...@linaro.org Date: Mon Jun 10 17:03:37 2013 +0200 mmc: core: Validate suspend prerequisites for SDIO at SUSPEND_PREPARE Changed this behaviour? For example, the libertas SDIO driver's suspend callback still returns -ENOSYS and before this commit, that error code will result in the SDIO device being removed; after this commit, that would result in an error code returned to PM core and a failure in system suspend. I'm not sure if I understand this correctly as I do not have any SDIO card to test. Can you please take a look at this? If this is indeed the case, do we need to maintain this behaviour? I need to know this answer as that would affect the way I'm going to solve my problem. Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: SDIO driver return -ENOSYS behaviour change?
On 02/27/2014 06:18 PM, Ulf Hansson wrote: On 27 February 2014 10:10, Aaron Lu aaron...@intel.com wrote: Hi Ulf, I was tracking some SDIO suspend problem and came across this. As Neil mentioned here: http://lkml.org/lkml/2012/3/25/20 Quote: SDIO (and possible MMC in general) has a protocol where the suspend method can return -ENOSYS and this means There is no point in suspending, just turn me off. It seems that the following commit: commit 810caddba42a54fe5db4e2664757a9a334ba359c Author: Ulf Hansson ulf.hans...@linaro.org Date: Mon Jun 10 17:03:37 2013 +0200 mmc: core: Validate suspend prerequisites for SDIO at SUSPEND_PREPARE Changed this behaviour? I realized I changed the behaviour to not cover for sdio func drivers, that actually implements the pm callbacks - but do return -ENOSYS in them. That wasn't obvious when looking at the code back then, sorry! Never mind, this behaviour change doesn't cause my problems but knowing whether this behaviour should be kept affects how I'm going to solve my problem. My problem is that, after the following commit: commit eed222aca8d077af3600b651176f6fd04d95cce1 Author: Aaron Lu aaron...@intel.com Date: Tue Mar 5 11:24:52 2013 +0800 mmc: sdio: bind acpi with sdio function device The SDIO function that has an ACPI node associated with will have the pm_domain assigned, which breaks the intention that during SDIO function device suspend phase nothing should be done by having a dummy BUS layer callback. The existence of the pm_domain for the SDIO function device will make its function driver's suspend callback gets called now. The end result is the function driver's suspend callback is called twice. To solve this problem, I was wondering why SDIO function device has this 'special' requirement that nothing should be done at its own device suspend phase but instead, relies on its suspend callback gets called in its parent device's suspend callback. And then I realized the reason is for the special handling of -ENOSYS. So if we could get rid of the -ENOSYS, my problem could be easily solved by deleting some lines in current code(the calling of function driver's suspend callback in mmc_sdio_suspend and the dummy system suspend/resume callback for SDIO bus). Buf if the behaviour has to be kept, I'll probably need to remove the pm_domain for the device and do some additional ACPI handing in mmc_sdio_suspend/resume for the function device. There are no solution to this problem in the mmc core right now, since we can't remove the card while we have reach the state when the suspend callback is being invoked. Instead, the sdio func driver shall not implement the PM callbacks at all. That behaviour means the mmc core will remove the card, but now it's done a in an earlier phase of the system suspend when we are still able to remove it. The libertas suspend callback is doing different things depending on different conditions - sometime it will enable wakeup capability and sometime it will want the mmc core to remove the device entirely by retuning -ENOSYS, so it may not be that easy by just deleting the callback, but I don't know for sure. Thanks, Aaron -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: SDIO driver return -ENOSYS behaviour change?
On 02/27/2014 09:05 PM, Ulf Hansson wrote:
On 27 February 2014 12:26, Aaron Lu aaron...@intel.com wrote:
On 02/27/2014 06:18 PM, Ulf Hansson wrote:
On 27 February 2014 10:10, Aaron Lu aaron...@intel.com wrote:
Hi Ulf,
I was tracking some SDIO suspend problem and came across this. As Neil
mentioned here:
http://lkml.org/lkml/2012/3/25/20
Quote:
SDIO (and possible MMC in general) has a protocol where the suspend
method can return -ENOSYS and this means There is no point in suspending,
just turn me off.
It seems that the following commit:
commit 810caddba42a54fe5db4e2664757a9a334ba359c
Author: Ulf Hansson ulf.hans...@linaro.org
Date: Mon Jun 10 17:03:37 2013 +0200
mmc: core: Validate suspend prerequisites for SDIO at SUSPEND_PREPARE
Changed this behaviour?
I realized I changed the behaviour to not cover for sdio func drivers,
that actually implements the pm callbacks - but do return -ENOSYS in
them. That wasn't obvious when looking at the code back then, sorry!
Never mind, this behaviour change doesn't cause my problems but knowing
whether this behaviour should be kept affects how I'm going to solve my
problem.
So let's aim for the a proper solution instead of a quick hack.
OK.
Although apparently mwifiex, libertas and btmrvl_sdio (bluetooth) may
return -ENOSYS from the respective system supend callbacks, thus
expecting the card to be removed. Currently this won't happen, but
instead the suspend will be aborted, which is really bad.
I believe those driver's suspend callback should be fixed to not
return -ENOSYS. Returning 0 will, when MMC_PM_KEEP_POWER is not set,
power off the card similar what's done when removing the card - that
should be perfectly fine. Do note that the sdio func driver then
should expect the resume callback to invoked, instead of being
_probed_ at the next system resume.
Good to know this.
My problem is that, after the following commit:
commit eed222aca8d077af3600b651176f6fd04d95cce1
Author: Aaron Lu aaron...@intel.com
Date: Tue Mar 5 11:24:52 2013 +0800
mmc: sdio: bind acpi with sdio function device
The SDIO function that has an ACPI node associated with will have the
pm_domain assigned, which breaks the intention that during SDIO function
device suspend phase nothing should be done by having a dummy BUS layer
callback. The existence of the pm_domain for the SDIO function device
will make its function driver's suspend callback gets called now. The
end result is the function driver's suspend callback is called twice.
Why is the sdio bus ignored?
In __device_suspend, if pm_domain is set, the suspend callback is
fetched there, no matter if there is a suspend callback defined in the
pm_domain or not. In ACPI PM domain's case, the suspend callback is
NULL, so the driver's suspend callback is used then.
I consulted Rafael whether adding a NULL check before goto Run is OK
like the following shows:
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 1b41fca3d65a..506583d84ed4 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1160,7 +1160,8 @@ static int __device_suspend(struct device *dev,
pm_message_t state, bool async)
if (dev-pm_domain) {
info = power domain ;
callback = pm_op(dev-pm_domain-ops, state);
- goto Run;
+ if (callback)
+ goto Run;
}
if (dev-type dev-type-pm) {
And Rafael said that would introduce regressions elsewhere, so it's a no
go.
Are you saying the power domain is using the pm_generic_suspend for or
from it's -suspend callback? If so, that could be the problem!?
To solve this problem, I was wondering why SDIO function device has this
'special' requirement that nothing should be done at its own device
suspend phase but instead, relies on its suspend callback gets called in
its parent device's suspend callback. And then I realized the reason is
for the special handling of -ENOSYS.
That's to my understanding not the only reason.
I think it's more a matter of having a controlled suspend sequence.
The mmc core are not able to serve any new SDIO requests while it is
suspended, therefore it tells the sdio func driver about when it safe
to send request - using it's PM callbacks.
Does this mean after the function device is suspended from PM core's
pespective, the mmc core will still send some requests to the function
device? I did see one such request, disable_width, get sent after the
function driver's suspend callback is invoked, don't know if there are
others.
From the mmc_sdio_suspend function I can see two things are done:
1 function device driver's suspend callback is called;
2 optionally disable_width and power off the card according to some flags.
So once we fixed the -ENOSYS problem, can we have the function device
run its own suspend callback and have the mmc_sdio_suspend just did the
disable_width
Re: [PATCH V2] sdhci: only reprogram retuning timer when flag is set
On 11/13/2013 07:25 PM, Dong Aisheng wrote:
On Wed, Nov 13, 2013 at 7:18 PM, Arend van Spriel ar...@broadcom.com wrote:
On 11/13/2013 12:12 PM, Dong Aisheng wrote:
Hi Arend,
On Wed, Nov 13, 2013 at 6:21 PM, Arend van Spriel ar...@broadcom.com
wrote:
On 11/13/2013 06:02 AM, Dong Aisheng wrote:
Hi Arend,
On Mon, Nov 11, 2013 at 5:49 PM, Arend van Spriel ar...@broadcom.com
wrote:
When the host-tuning_count is zero it means that the
retuning is disabled. This is checked on the first
run of sdhci_execute_tuning() by the if statement below:
if (!(host-flags SDHCI_NEEDS_RETUNING)
host-tuning_count
(host-tuning_mode == SDHCI_TUNING_MODE_1)) {
So only when tuning_count is non-zero it will set the host
flag SDHCI_USING_RETUNING_TIMER. The else statement is only
for re-programming the timer, which means that flag must be
set. Because that is not checked the else statement is executed
in the first run when tuning_count is zero.
This was seen on a host controller which indicated
SDHCI_TUNING_MODE_1 (0) and tuning_count being zero. Suspect
that (one of) these registers is not properly set.
Signed-off-by: Arend van Spriel ar...@broadcom.com
---
This patch applies to the mmc-next branch.
V2:
- add more explanation to the commit message
- check host flag SDHCI_USING_RETUNING_TIMER
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index bd8a098..5974599 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2014,7 +2014,7 @@ out:
host-tuning_count * HZ);
/* Tuning mode 1 limits the maximum data length to
4MB
*/
mmc-max_blk_count = (4 * 1024 * 1024) /
mmc-max_blk_size;
- } else {
+ } else if (host-flags SDHCI_USING_RETUNING_TIMER) {
host-flags = ~SDHCI_NEEDS_RETUNING;
/* Reload the new initial value for timer */
if (host-tuning_mode == SDHCI_TUNING_MODE_1)
I wonder if we could also remove this line?
It looks to me it's not neccesary to check the tuning_mode again since
we already check the flag
above and SDHCI_TUNING_MODE_1 seems like the prerequisite of
SDHCI_USING_RETUNING_TIMER.
According the spec the other tuning modes also can use retuning timer.
Currently, the mmc stack in upstream linux only supports tuning mode 1.
When
adding the other modes this if statement will probably go.
For currently code, it looks like also not necessary to check it since
SDHCI_USING_RETUNING_TIMER will only be set when tunning_mode is
SDHCI_TUNING_MODE_1.
And SDHCI_TUNING_MODE_1 just indicates the tuning mode while the flag
SDHCI_USING_RETUNING_TIMER represents the retuning timer implementation.
So check the flag to invoke the timer seems make more sense to me.
do you agree?
The flag SDHCI_USING_RETUNING_TIMER is only set after the initial tuning run
so in the if-statement. So currently in the else-statement the fact that
SDHCI_USING_RETUNING_TIMER is set implies SDHCI_TUNING_MODE_1.
Right, so that means we could remove the tuning_mode check in the
else-statement.
I agree.
-Aaron
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 2d55e6a..b2928ef 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2015,12 +2015,11 @@ out:
host-tuning_count * HZ);
/* Tuning mode 1 limits the maximum data length to 4MB */
mmc-max_blk_count = (4 * 1024 * 1024) / mmc-max_blk_size;
- } else {
+ } else if (host-flags SDHCI_USING_RETUNING_TIMER) {
host-flags = ~SDHCI_NEEDS_RETUNING;
/* Reload the new initial value for timeout workqueue */
- if (host-tuning_mode == SDHCI_TUNING_MODE_1)
- schedule_delayed_work(host-tuning_timeout_work,
- host-tuning_count * HZ);
+ schedule_delayed_work(host-tuning_timeout_work,
+ host-tuning_count * HZ);
}
/*
Regards
Dong Aisheng
Regards,
Arend
Regards
Dong Aisheng
Regards,
Arend
Regards
Dong Aisheng
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
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 majord...@vger.kernel.org
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] mmc: sdhci: supporting PCI MSI
On 11/13/2013 02:00 PM, Jackey Shen wrote:
On Wed, Nov 13, 2013 at 01:55:08PM +0800, Jackey Shen wrote:
What about we only call pci_enable_msi in sdhci-pci.c and then assign
host-irq appropriately before calling sdhci_add_host, will that work?
The only difference would be, request_irq will have SHARED flag, but I
suppose that's not a problem.
There are 2 points for this part:
1. fall back to legacy interrupt right after MSI request fails;
If we call pci_enable_msi somewhere in sdhci_pci_probe_slot, then if it
failed, we can also fall back I suppose?
Yes, it is.
But, sdhci_pci_disable_msi should be kept since pci_disable_msi is
pci bus releted and better not used in sdhci.c.
So, enable msi and disable msi are NOT symmetric in style.
What's your opinion?
Sorry for missing word.
So, enable msi and disable msi are NOT symmetric in style.
I meant something like this:
diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
index d7d6bc8..96461a3 100644
--- a/drivers/mmc/host/sdhci-pci.c
+++ b/drivers/mmc/host/sdhci-pci.c
@@ -1339,12 +1339,17 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
host-quirks = chip-quirks;
host-quirks2 = chip-quirks2;
+ ret = pci_enable_msi(pdev);
+ if (ret) {
+ dev_err(pdev-dev, cannot enable msi\n);
+ goto cleanup;
+ }
host-irq = pdev-irq;
ret = pci_request_region(pdev, bar, mmc_hostname(host-mmc));
if (ret) {
dev_err(pdev-dev, cannot request region\n);
- goto cleanup;
+ goto disable_msi;
}
host-ioaddr = pci_ioremap_bar(pdev, bar);
@@ -1396,6 +1401,9 @@ unmap:
release:
pci_release_region(pdev, bar);
+disable_msi:
+ pci_disable_msi(pdev);
+
cleanup:
if (slot-data slot-data-cleanup)
slot-data-cleanup(slot-data);
@@ -1431,6 +1439,8 @@ static void sdhci_pci_remove_slot(struct sdhci_pci_slot
*slot)
pci_release_region(slot-chip-pdev, slot-pci_bar);
+ pci_disable_msi(slot-chip-pdev);
+
sdhci_free_host(slot-host);
}
We can do all these MSI stuffs in sdhci-pci.c, but I'm not sure if this
is correct.
Thanks,
Aaron
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] mmc: sdhci: supporting PCI MSI
On 11/12/2013 05:27 PM, Jackey Shen wrote:
On Tue, Nov 12, 2013 at 03:44:40PM +0800, Aaron Lu wrote:
On 11/12/2013 02:56 PM, Jackey Shen wrote:
Enable MSI support in sdhci-pci driver and provide the mechanism
to fall back to Legacy Pin-based Interrupt if MSI register fails.
Tested with SD cards on AMD platform.
-
Change from V1:
- implement this PCI MSI feature in sdhci-pci.c instead of sdhci.c
Such words can be placed under the --- below so that it will not appear
in git log.
Signed-off-by: Jackey Shen jackey.s...@amd.com
Reviewed-by: Huang Rui ray.hu...@amd.com
---
-
Put change related info Here.
OK. I will update my patch.
drivers/mmc/host/Kconfig | 11 +
drivers/mmc/host/sdhci-pci.c | 58
drivers/mmc/host/sdhci.c | 42 ++--
drivers/mmc/host/sdhci.h | 2 ++
include/linux/mmc/sdhci.h| 2 ++
5 files changed, 107 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 7fc5099..a56cf27 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -68,6 +68,17 @@ config MMC_SDHCI_PCI
If unsure, say N.
+config MMC_SDHCI_PCI_MSI
+ bool SDHCI support with MSI on PCI bus
+ depends on MMC_SDHCI_PCI PCI_MSI
+ help
+ This enables PCI MSI (Message Signaled Interrupts) for Secure
+ Digital Host Controller Interface.
+
+ If you have a controller with this capability, say Y or M here.
+
+ If unsure, say N.
+
config MMC_RICOH_MMC
bool Ricoh MMC Controller Disabler
depends on MMC_SDHCI_PCI
diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
index 8f75381..2ca29c0 100644
--- a/drivers/mmc/host/sdhci-pci.c
+++ b/drivers/mmc/host/sdhci-pci.c
@@ -1143,10 +1143,68 @@ static void sdhci_pci_hw_reset(struct sdhci_host
*host)
slot-hw_reset(host);
}
+#ifdef CONFIG_MMC_SDHCI_PCI_MSI
+static int sdhci_pci_setup_msi(struct sdhci_host *host, irq_handler_t
handler)
+{
+ int ret;
+ struct pci_dev *pdev = to_pci_dev(host-mmc-parent);
+
+ host-msi_enabled = false;
+
+ ret = pci_enable_msi(pdev);
+ if (ret) {
+ pr_warn(%s: Failed to allocate MSI entry\n,
+ mmc_hostname(host-mmc));
+ return ret;
+ }
+
+ ret = request_irq(pdev-irq, handler, 0,
+ mmc_hostname(host-mmc), host);
+ if (ret) {
+ pr_warn(%s: Failed to request MSI IRQ %d: %d\n,
+ mmc_hostname(host-mmc), pdev-irq, ret);
+ pci_disable_msi(pdev);
+ return ret;
+ }
+
+ host-irq = pdev-irq;
+ host-msi_enabled = true;
+ return ret;
+}
What about we only call pci_enable_msi in sdhci-pci.c and then assign
host-irq appropriately before calling sdhci_add_host, will that work?
The only difference would be, request_irq will have SHARED flag, but I
suppose that's not a problem.
There are 2 points for this part:
1. fall back to legacy interrupt right after MSI request fails;
If we call pci_enable_msi somewhere in sdhci_pci_probe_slot, then if it
failed, we can also fall back I suppose?
2. prompt user more information about where error/warning occur.
+
+static int sdhci_pci_enable_msi(struct sdhci_host *host, irq_handler_t
handler)
+{
+ return sdhci_pci_setup_msi(host, handler);
+}
+
+static void sdhci_pci_disable_msi(struct sdhci_host *host)
+{
+ struct pci_dev *pdev = to_pci_dev(host-mmc-parent);
+
+ if (host-msi_enabled) {
+ pci_disable_msi(pdev);
+ host-msi_enabled = false;
+ }
+}
+
+#else
+
+static int sdhci_pci_enable_msi(struct sdhci_host *host, irq_handler_t
handler)
+{
+ return 0;
+}
+
+static void sdhci_pci_disable_msi(struct sdhci_host *host)
+{
+}
+#endif /* CONFIG_MMC_SDHCI_PCI_MSI */
+
static const struct sdhci_ops sdhci_pci_ops = {
.enable_dma = sdhci_pci_enable_dma,
.platform_bus_width = sdhci_pci_bus_width,
.hw_reset = sdhci_pci_hw_reset,
+ .enable_msi = sdhci_pci_enable_msi,
+ .disable_msi= sdhci_pci_disable_msi,
};
/*\
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6785fb1..4c2a1ac 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2554,6 +2554,31 @@ void sdhci_disable_irq_wakeups(struct sdhci_host
*host)
}
EXPORT_SYMBOL_GPL(sdhci_disable_irq_wakeups);
+static int sdhci_request_irq(struct sdhci_host *host)
+{
+ int ret = 0;
+
+ /* try to enable MSI */
+ if (host-ops-enable_msi) {
+ ret = host-ops-enable_msi(host, sdhci_irq);
+ if (ret)
+ pr_warn(%s: Fall back to Pin-based Interrupt: %d\n,
+ mmc_hostname(host-mmc), ret);
+ }
+
+ if (!host-msi_enabled
Re: [PATCH] mmc: sdhci: supporting PCI MSI
On 11/01/2013 05:45 PM, Jackey Shen wrote:
Enable MSI support in sdhci-pci driver and provide the mechanism
to fall back to Legacy Pin-based Interrupt if MSI register fails.
Also note the following thread:
http://marc.info/?l=linux-mmcm=133346937412708w=2
Thanks,
Aaron
Signed-off-by: Jackey Shen jackey.s...@amd.com
Reviewed-by: Huang Rui ray.hu...@amd.com
---
drivers/mmc/host/Kconfig | 11 +
drivers/mmc/host/sdhci.c | 108
--
include/linux/mmc/sdhci.h | 2 +
3 files changed, 109 insertions(+), 12 deletions(-)
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 7fc5099..a56cf27 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -68,6 +68,17 @@ config MMC_SDHCI_PCI
If unsure, say N.
+config MMC_SDHCI_PCI_MSI
+ bool SDHCI support with MSI on PCI bus
+ depends on MMC_SDHCI_PCI PCI_MSI
+ help
+ This enables PCI MSI (Message Signaled Interrupts) for Secure
+ Digital Host Controller Interface.
+
+ If you have a controller with this capability, say Y or M here.
+
+ If unsure, say N.
+
config MMC_RICOH_MMC
bool Ricoh MMC Controller Disabler
depends on MMC_SDHCI_PCI
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6785fb1..de509bc 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -13,6 +13,10 @@
* - JMicron (hardware and technical support)
*/
+#ifdef CONFIG_MMC_SDHCI_PCI_MSI
+#include linux/pci.h
+#endif
+
#include linux/delay.h
#include linux/highmem.h
#include linux/io.h
@@ -2520,6 +2524,64 @@ out:
return result;
}
+#ifdef CONFIG_MMC_SDHCI_PCI_MSI
+static int sdhci_setup_msi(struct sdhci_host *host)
+{
+ int ret;
+ struct pci_dev *pdev = to_pci_dev(host-mmc-parent);
+
+ host-msi_enabled = false;
+
+ ret = pci_enable_msi(pdev);
+ if (ret) {
+ pr_warn(%s: Failed to allocate MSI entry\n,
+ mmc_hostname(host-mmc));
+ return ret;
+ }
+
+ ret = request_irq(pdev-irq, sdhci_irq, 0,
+ mmc_hostname(host-mmc), host);
+ if (ret) {
+ pr_warn(%s: Failed to request MSI IRQ %d: %d\n,
+mmc_hostname(host-mmc), pdev-irq, ret);
+ pci_disable_msi(pdev);
+ return ret;
+ }
+
+ host-irq = pdev-irq;
+ host-msi_enabled = true;
+ return ret;
+}
+
+static int sdhci_try_enable_msi(struct sdhci_host *host)
+{
+ return sdhci_setup_msi(host);
+}
+
+static void sdhci_cleanup_msi(struct sdhci_host *host)
+{
+ struct pci_dev *pdev = to_pci_dev(host-mmc-parent);
+
+ free_irq(host-irq, host);
+ if (host-msi_enabled) {
+ pci_disable_msi(pdev);
+ host-msi_enabled = false;
+ }
+}
+
+#else
+
+static int sdhci_try_enable_msi(struct sdhci_host *host)
+{
+ return 0;
+}
+
+static void sdhci_cleanup_msi(struct sdhci_host *host)
+{
+ free_irq(host-irq, host);
+}
+#endif /* CONFIG_MMC_SDHCI_PCI_MSI */
+
/*\
*
*
* Suspend/resume
*
@@ -2569,7 +2631,8 @@ int sdhci_suspend_host(struct sdhci_host *host)
if (!device_may_wakeup(mmc_dev(host-mmc))) {
sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
- free_irq(host-irq, host);
+ /* free any IRQ and disable MSI */
+ sdhci_cleanup_msi(host);
} else {
sdhci_enable_irq_wakeups(host);
enable_irq_wake(host-irq);
@@ -2589,10 +2652,22 @@ int sdhci_resume_host(struct sdhci_host *host)
}
if (!device_may_wakeup(mmc_dev(host-mmc))) {
- ret = request_irq(host-irq, sdhci_irq, IRQF_SHARED,
- mmc_hostname(host-mmc), host);
+ /* try to enable MSI */
+ ret = sdhci_try_enable_msi(host);
if (ret)
- return ret;
+ pr_warn(%s: Fall back to Legacy Pin-based Interrupt:
%d\n,
+ mmc_hostname(host-mmc), ret);
+
+ if (!host-msi_enabled) {
+ /* fall back to legacy pin-based interrupt */
+ ret = request_irq(host-irq, sdhci_irq, IRQF_SHARED,
+ mmc_hostname(host-mmc), host);
+ if (ret) {
+ pr_err(%s: Failed to request IRQ %d: %d\n,
+ mmc_hostname(host-mmc), host-irq,
ret);
+ return ret;
+ }
+ }
} else {
sdhci_disable_irq_wakeups(host);
Re: [PATCH V2] sdhci: only reprogram retuning timer when flag is set
On 11/11/2013 05:49 PM, Arend van Spriel wrote:
When the host-tuning_count is zero it means that the
retuning is disabled. This is checked on the first
run of sdhci_execute_tuning() by the if statement below:
if (!(host-flags SDHCI_NEEDS_RETUNING) host-tuning_count
(host-tuning_mode == SDHCI_TUNING_MODE_1)) {
So only when tuning_count is non-zero it will set the host
flag SDHCI_USING_RETUNING_TIMER. The else statement is only
for re-programming the timer, which means that flag must be
set. Because that is not checked the else statement is executed
in the first run when tuning_count is zero.
This was seen on a host controller which indicated
SDHCI_TUNING_MODE_1 (0) and tuning_count being zero. Suspect
that (one of) these registers is not properly set.
Signed-off-by: Arend van Spriel ar...@broadcom.com
Reviewed-by: Aaron Lu aaron...@intel.com
-Aaron
---
This patch applies to the mmc-next branch.
V2:
- add more explanation to the commit message
- check host flag SDHCI_USING_RETUNING_TIMER
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index bd8a098..5974599 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2014,7 +2014,7 @@ out:
host-tuning_count * HZ);
/* Tuning mode 1 limits the maximum data length to 4MB */
mmc-max_blk_count = (4 * 1024 * 1024) / mmc-max_blk_size;
- } else {
+ } else if (host-flags SDHCI_USING_RETUNING_TIMER) {
host-flags = ~SDHCI_NEEDS_RETUNING;
/* Reload the new initial value for timer */
if (host-tuning_mode == SDHCI_TUNING_MODE_1)
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] mmc: sdhci: supporting PCI MSI
On 11/12/2013 02:56 PM, Jackey Shen wrote:
Enable MSI support in sdhci-pci driver and provide the mechanism
to fall back to Legacy Pin-based Interrupt if MSI register fails.
Tested with SD cards on AMD platform.
-
Change from V1:
- implement this PCI MSI feature in sdhci-pci.c instead of sdhci.c
Such words can be placed under the --- below so that it will not appear
in git log.
Signed-off-by: Jackey Shen jackey.s...@amd.com
Reviewed-by: Huang Rui ray.hu...@amd.com
---
-
Put change related info Here.
drivers/mmc/host/Kconfig | 11 +
drivers/mmc/host/sdhci-pci.c | 58
drivers/mmc/host/sdhci.c | 42 ++--
drivers/mmc/host/sdhci.h | 2 ++
include/linux/mmc/sdhci.h| 2 ++
5 files changed, 107 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 7fc5099..a56cf27 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -68,6 +68,17 @@ config MMC_SDHCI_PCI
If unsure, say N.
+config MMC_SDHCI_PCI_MSI
+ bool SDHCI support with MSI on PCI bus
+ depends on MMC_SDHCI_PCI PCI_MSI
+ help
+ This enables PCI MSI (Message Signaled Interrupts) for Secure
+ Digital Host Controller Interface.
+
+ If you have a controller with this capability, say Y or M here.
+
+ If unsure, say N.
+
config MMC_RICOH_MMC
bool Ricoh MMC Controller Disabler
depends on MMC_SDHCI_PCI
diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
index 8f75381..2ca29c0 100644
--- a/drivers/mmc/host/sdhci-pci.c
+++ b/drivers/mmc/host/sdhci-pci.c
@@ -1143,10 +1143,68 @@ static void sdhci_pci_hw_reset(struct sdhci_host
*host)
slot-hw_reset(host);
}
+#ifdef CONFIG_MMC_SDHCI_PCI_MSI
+static int sdhci_pci_setup_msi(struct sdhci_host *host, irq_handler_t
handler)
+{
+ int ret;
+ struct pci_dev *pdev = to_pci_dev(host-mmc-parent);
+
+ host-msi_enabled = false;
+
+ ret = pci_enable_msi(pdev);
+ if (ret) {
+ pr_warn(%s: Failed to allocate MSI entry\n,
+ mmc_hostname(host-mmc));
+ return ret;
+ }
+
+ ret = request_irq(pdev-irq, handler, 0,
+ mmc_hostname(host-mmc), host);
+ if (ret) {
+ pr_warn(%s: Failed to request MSI IRQ %d: %d\n,
+mmc_hostname(host-mmc), pdev-irq, ret);
+ pci_disable_msi(pdev);
+ return ret;
+ }
+
+ host-irq = pdev-irq;
+ host-msi_enabled = true;
+ return ret;
+}
What about we only call pci_enable_msi in sdhci-pci.c and then assign
host-irq appropriately before calling sdhci_add_host, will that work?
The only difference would be, request_irq will have SHARED flag, but I
suppose that's not a problem.
+
+static int sdhci_pci_enable_msi(struct sdhci_host *host, irq_handler_t
handler)
+{
+ return sdhci_pci_setup_msi(host, handler);
+}
+
+static void sdhci_pci_disable_msi(struct sdhci_host *host)
+{
+ struct pci_dev *pdev = to_pci_dev(host-mmc-parent);
+
+ if (host-msi_enabled) {
+ pci_disable_msi(pdev);
+ host-msi_enabled = false;
+ }
+}
+
+#else
+
+static int sdhci_pci_enable_msi(struct sdhci_host *host, irq_handler_t
handler)
+{
+ return 0;
+}
+
+static void sdhci_pci_disable_msi(struct sdhci_host *host)
+{
+}
+#endif /* CONFIG_MMC_SDHCI_PCI_MSI */
+
static const struct sdhci_ops sdhci_pci_ops = {
.enable_dma = sdhci_pci_enable_dma,
.platform_bus_width = sdhci_pci_bus_width,
.hw_reset = sdhci_pci_hw_reset,
+ .enable_msi = sdhci_pci_enable_msi,
+ .disable_msi= sdhci_pci_disable_msi,
};
/*\
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6785fb1..4c2a1ac 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2554,6 +2554,31 @@ void sdhci_disable_irq_wakeups(struct sdhci_host *host)
}
EXPORT_SYMBOL_GPL(sdhci_disable_irq_wakeups);
+static int sdhci_request_irq(struct sdhci_host *host)
+{
+ int ret = 0;
+
+ /* try to enable MSI */
+ if (host-ops-enable_msi) {
+ ret = host-ops-enable_msi(host, sdhci_irq);
+ if (ret)
+ pr_warn(%s: Fall back to Pin-based Interrupt: %d\n,
+ mmc_hostname(host-mmc), ret);
+ }
+
+ if (!host-msi_enabled) {
+ /* fall back to legacy pin-based interrupt */
+ ret = request_irq(host-irq, sdhci_irq, IRQF_SHARED,
+ mmc_hostname(host-mmc), host);
+ if (ret) {
+ pr_err(%s: Failed to request IRQ %d: %d\n,
+
Re: [PATCH] mmc: sdhci: supporting PCI MSI
On 11/08/2013 04:05 PM, Jackey Shen wrote:
Hi Aaron,
On Fri, Nov 08, 2013 at 02:58:45PM +0800, Aaron Lu wrote:
On 11/01/2013 05:45 PM, Jackey Shen wrote:
Enable MSI support in sdhci-pci driver and provide the mechanism
to fall back to Legacy Pin-based Interrupt if MSI register fails.
Signed-off-by: Jackey Shen jackey.s...@amd.com
Reviewed-by: Huang Rui ray.hu...@amd.com
---
drivers/mmc/host/Kconfig | 11 +
drivers/mmc/host/sdhci.c | 108
--
include/linux/mmc/sdhci.h | 2 +
3 files changed, 109 insertions(+), 12 deletions(-)
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 7fc5099..a56cf27 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -68,6 +68,17 @@ config MMC_SDHCI_PCI
If unsure, say N.
+config MMC_SDHCI_PCI_MSI
+ bool SDHCI support with MSI on PCI bus
+ depends on MMC_SDHCI_PCI PCI_MSI
+ help
+ This enables PCI MSI (Message Signaled Interrupts) for Secure
+ Digital Host Controller Interface.
+
+ If you have a controller with this capability, say Y or M here.
+
+ If unsure, say N.
+
config MMC_RICOH_MMC
bool Ricoh MMC Controller Disabler
depends on MMC_SDHCI_PCI
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6785fb1..de509bc 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
I think PCI stuffs should be in sdhci-pci.c, not sdhci.c.
But I ran into one issue as per your suggestion:
sdhci_add|remove_host() in sdhci.c calls back sdhci_try_enable|cleanup()
in_msi sdhci-pci.c if sdhci_try_enable|cleanup_msi() are moved into
sdhci-pci.c.
I don't know much about PCI and interrupt, but what is the problem of
doing enable/disable msi in sdhci_pci_probe_slot?
Thanks,
Aaron
Is it a best practice?
Thanks,
Jackey
Thanks,
Aaron
@@ -13,6 +13,10 @@
* - JMicron (hardware and technical support)
*/
+#ifdef CONFIG_MMC_SDHCI_PCI_MSI
+#include linux/pci.h
+#endif
+
#include linux/delay.h
#include linux/highmem.h
#include linux/io.h
@@ -2520,6 +2524,64 @@ out:
return result;
}
+#ifdef CONFIG_MMC_SDHCI_PCI_MSI
+static int sdhci_setup_msi(struct sdhci_host *host)
+{
+ int ret;
+ struct pci_dev *pdev = to_pci_dev(host-mmc-parent);
+
+ host-msi_enabled = false;
+
+ ret = pci_enable_msi(pdev);
+ if (ret) {
+ pr_warn(%s: Failed to allocate MSI entry\n,
+ mmc_hostname(host-mmc));
+ return ret;
+ }
+
+ ret = request_irq(pdev-irq, sdhci_irq, 0,
+ mmc_hostname(host-mmc), host);
+ if (ret) {
+ pr_warn(%s: Failed to request MSI IRQ %d: %d\n,
+ mmc_hostname(host-mmc), pdev-irq, ret);
+ pci_disable_msi(pdev);
+ return ret;
+ }
+
+ host-irq = pdev-irq;
+ host-msi_enabled = true;
+ return ret;
+}
+
+static int sdhci_try_enable_msi(struct sdhci_host *host)
+{
+ return sdhci_setup_msi(host);
+}
+
+static void sdhci_cleanup_msi(struct sdhci_host *host)
+{
+ struct pci_dev *pdev = to_pci_dev(host-mmc-parent);
+
+ free_irq(host-irq, host);
+ if (host-msi_enabled) {
+ pci_disable_msi(pdev);
+ host-msi_enabled = false;
+ }
+}
+
+#else
+
+static int sdhci_try_enable_msi(struct sdhci_host *host)
+{
+ return 0;
+}
+
+static void sdhci_cleanup_msi(struct sdhci_host *host)
+{
+ free_irq(host-irq, host);
+}
+#endif /* CONFIG_MMC_SDHCI_PCI_MSI */
+
/*\
*
*
* Suspend/resume
*
@@ -2569,7 +2631,8 @@ int sdhci_suspend_host(struct sdhci_host *host)
if (!device_may_wakeup(mmc_dev(host-mmc))) {
sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
- free_irq(host-irq, host);
+ /* free any IRQ and disable MSI */
+ sdhci_cleanup_msi(host);
} else {
sdhci_enable_irq_wakeups(host);
enable_irq_wake(host-irq);
@@ -2589,10 +2652,22 @@ int sdhci_resume_host(struct sdhci_host *host)
}
if (!device_may_wakeup(mmc_dev(host-mmc))) {
- ret = request_irq(host-irq, sdhci_irq, IRQF_SHARED,
- mmc_hostname(host-mmc), host);
+ /* try to enable MSI */
+ ret = sdhci_try_enable_msi(host);
if (ret)
- return ret;
+ pr_warn(%s: Fall back to Legacy Pin-based Interrupt:
%d\n,
+ mmc_hostname(host-mmc), ret);
+
+ if (!host-msi_enabled) {
+ /* fall back to legacy pin-based interrupt */
+ ret = request_irq(host-irq, sdhci_irq, IRQF_SHARED,
+ mmc_hostname(host-mmc), host
Re: [PATCH] sdhci: do not program timer when tuning_count is zero
On 11/08/2013 05:10 PM, Arend van Spriel wrote:
On 11/08/2013 07:50 AM, Aaron Lu wrote:
On 11/07/2013 06:59 PM, Arend van Spriel wrote:
When the host-tuning_count is zero it means that the
If the tuning_count is zero, then the retuning timer shouldn't be
started in the first place and not possible to run code there. Or is the
tuning_count dynamically changed?
Actually, the sdhci_execute_tuning() must run once to do the initial
tuning procedure. This is mandatory for SDR104. However, *re*tuning is
not and a zero tuning_count disables it.
So the host in question doesn't need do retuning while in SDR104 mode?
It seems your host's retuning mode is mode_1.
The function is executed initially. The 'if' statement above the patched
'else' statement is actually responsible for programming the retuning
timer for the first time. However, it requires tuning_count to be
non-zero. The 'else' statement is actually for reloading the retuning
timer, which is not the case. Adding the non-zero check assures the
retuning timer is never started.
OK, I see.
The SDHCI_USING_RETUNING_TIMER flag is supposed to mean if the host is
currently using retuning timer to do retuning, it could also be used to
decide if retuning timer needs be re-programmed.
Anyway, a host in retuning mode 1 does not have tuning_count set seems a
little odd to me...
Thanks,
Aaron
I guess the fact that this needs explaining indicates that the commit
message should be updated. I will send a V2 for this.
Regards,
Arend
Thanks,
Aaron
retuning is disabled. Doing a mod_timer() with a zero
tuning_count does something else.
Signed-off-by: Arend van Spriel ar...@broadcom.com
---
drivers/mmc/host/sdhci.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 7a7fb4f..9803e7a 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2007,7 +2007,8 @@ out:
} else {
host-flags = ~SDHCI_NEEDS_RETUNING;
/* Reload the new initial value for timer */
- if (host-tuning_mode == SDHCI_TUNING_MODE_1)
+ if (host-tuning_count
+ host-tuning_mode == SDHCI_TUNING_MODE_1)
mod_timer(host-tuning_timer, jiffies +
host-tuning_count * HZ);
}
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sdhci: do not program timer when tuning_count is zero
On 11/08/2013 07:55 PM, Arend van Spriel wrote:
On 11/08/2013 12:49 PM, Aaron Lu wrote:
On 11/08/2013 05:10 PM, Arend van Spriel wrote:
On 11/08/2013 07:50 AM, Aaron Lu wrote:
On 11/07/2013 06:59 PM, Arend van Spriel wrote:
When the host-tuning_count is zero it means that the
If the tuning_count is zero, then the retuning timer shouldn't be
started in the first place and not possible to run code there. Or is the
tuning_count dynamically changed?
Actually, the sdhci_execute_tuning() must run once to do the initial
tuning procedure. This is mandatory for SDR104. However, *re*tuning is
not and a zero tuning_count disables it.
So the host in question doesn't need do retuning while in SDR104 mode?
It seems your host's retuning mode is mode_1.
The function is executed initially. The 'if' statement above the patched
'else' statement is actually responsible for programming the retuning
timer for the first time. However, it requires tuning_count to be
non-zero. The 'else' statement is actually for reloading the retuning
timer, which is not the case. Adding the non-zero check assures the
retuning timer is never started.
OK, I see.
The SDHCI_USING_RETUNING_TIMER flag is supposed to mean if the host is
currently using retuning timer to do retuning, it could also be used to
decide if retuning timer needs be re-programmed.
True. I can go for that approach.
Anyway, a host in retuning mode 1 does not have tuning_count set seems a
little odd to me...
Looking at sdhci.h it actually seems sdhci code is missing support for
the other modes:
unsigned inttuning_count; /* Timer count for re-tuning */
unsigned inttuning_mode;/* Re-tuning mode supported by
host */
#define SDHCI_TUNING_MODE_1 0
struct timer_list tuning_timer; /* Timer for tuning */
At least the other modes are not defined here.
Right, only retuning mode 1 is implemented currently.
Thanks,
Aaron
Regards,
Arend
Thanks,
Aaron
I guess the fact that this needs explaining indicates that the commit
message should be updated. I will send a V2 for this.
Regards,
Arend
Thanks,
Aaron
retuning is disabled. Doing a mod_timer() with a zero
tuning_count does something else.
Signed-off-by: Arend van Spriel ar...@broadcom.com
---
drivers/mmc/host/sdhci.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 7a7fb4f..9803e7a 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2007,7 +2007,8 @@ out:
} else {
host-flags = ~SDHCI_NEEDS_RETUNING;
/* Reload the new initial value for timer */
- if (host-tuning_mode == SDHCI_TUNING_MODE_1)
+ if (host-tuning_count
+ host-tuning_mode == SDHCI_TUNING_MODE_1)
mod_timer(host-tuning_timer, jiffies +
host-tuning_count * HZ);
}
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sdhci: do not program timer when tuning_count is zero
On 11/07/2013 06:59 PM, Arend van Spriel wrote:
When the host-tuning_count is zero it means that the
If the tuning_count is zero, then the retuning timer shouldn't be
started in the first place and not possible to run code there. Or is the
tuning_count dynamically changed?
Thanks,
Aaron
retuning is disabled. Doing a mod_timer() with a zero
tuning_count does something else.
Signed-off-by: Arend van Spriel ar...@broadcom.com
---
drivers/mmc/host/sdhci.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 7a7fb4f..9803e7a 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2007,7 +2007,8 @@ out:
} else {
host-flags = ~SDHCI_NEEDS_RETUNING;
/* Reload the new initial value for timer */
- if (host-tuning_mode == SDHCI_TUNING_MODE_1)
+ if (host-tuning_count
+ host-tuning_mode == SDHCI_TUNING_MODE_1)
mod_timer(host-tuning_timer, jiffies +
host-tuning_count * HZ);
}
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: sdhci: supporting PCI MSI
On 11/01/2013 05:45 PM, Jackey Shen wrote:
Enable MSI support in sdhci-pci driver and provide the mechanism
to fall back to Legacy Pin-based Interrupt if MSI register fails.
Signed-off-by: Jackey Shen jackey.s...@amd.com
Reviewed-by: Huang Rui ray.hu...@amd.com
---
drivers/mmc/host/Kconfig | 11 +
drivers/mmc/host/sdhci.c | 108
--
include/linux/mmc/sdhci.h | 2 +
3 files changed, 109 insertions(+), 12 deletions(-)
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 7fc5099..a56cf27 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -68,6 +68,17 @@ config MMC_SDHCI_PCI
If unsure, say N.
+config MMC_SDHCI_PCI_MSI
+ bool SDHCI support with MSI on PCI bus
+ depends on MMC_SDHCI_PCI PCI_MSI
+ help
+ This enables PCI MSI (Message Signaled Interrupts) for Secure
+ Digital Host Controller Interface.
+
+ If you have a controller with this capability, say Y or M here.
+
+ If unsure, say N.
+
config MMC_RICOH_MMC
bool Ricoh MMC Controller Disabler
depends on MMC_SDHCI_PCI
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6785fb1..de509bc 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
I think PCI stuffs should be in sdhci-pci.c, not sdhci.c.
Thanks,
Aaron
@@ -13,6 +13,10 @@
* - JMicron (hardware and technical support)
*/
+#ifdef CONFIG_MMC_SDHCI_PCI_MSI
+#include linux/pci.h
+#endif
+
#include linux/delay.h
#include linux/highmem.h
#include linux/io.h
@@ -2520,6 +2524,64 @@ out:
return result;
}
+#ifdef CONFIG_MMC_SDHCI_PCI_MSI
+static int sdhci_setup_msi(struct sdhci_host *host)
+{
+ int ret;
+ struct pci_dev *pdev = to_pci_dev(host-mmc-parent);
+
+ host-msi_enabled = false;
+
+ ret = pci_enable_msi(pdev);
+ if (ret) {
+ pr_warn(%s: Failed to allocate MSI entry\n,
+ mmc_hostname(host-mmc));
+ return ret;
+ }
+
+ ret = request_irq(pdev-irq, sdhci_irq, 0,
+ mmc_hostname(host-mmc), host);
+ if (ret) {
+ pr_warn(%s: Failed to request MSI IRQ %d: %d\n,
+mmc_hostname(host-mmc), pdev-irq, ret);
+ pci_disable_msi(pdev);
+ return ret;
+ }
+
+ host-irq = pdev-irq;
+ host-msi_enabled = true;
+ return ret;
+}
+
+static int sdhci_try_enable_msi(struct sdhci_host *host)
+{
+ return sdhci_setup_msi(host);
+}
+
+static void sdhci_cleanup_msi(struct sdhci_host *host)
+{
+ struct pci_dev *pdev = to_pci_dev(host-mmc-parent);
+
+ free_irq(host-irq, host);
+ if (host-msi_enabled) {
+ pci_disable_msi(pdev);
+ host-msi_enabled = false;
+ }
+}
+
+#else
+
+static int sdhci_try_enable_msi(struct sdhci_host *host)
+{
+ return 0;
+}
+
+static void sdhci_cleanup_msi(struct sdhci_host *host)
+{
+ free_irq(host-irq, host);
+}
+#endif /* CONFIG_MMC_SDHCI_PCI_MSI */
+
/*\
*
*
* Suspend/resume
*
@@ -2569,7 +2631,8 @@ int sdhci_suspend_host(struct sdhci_host *host)
if (!device_may_wakeup(mmc_dev(host-mmc))) {
sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
- free_irq(host-irq, host);
+ /* free any IRQ and disable MSI */
+ sdhci_cleanup_msi(host);
} else {
sdhci_enable_irq_wakeups(host);
enable_irq_wake(host-irq);
@@ -2589,10 +2652,22 @@ int sdhci_resume_host(struct sdhci_host *host)
}
if (!device_may_wakeup(mmc_dev(host-mmc))) {
- ret = request_irq(host-irq, sdhci_irq, IRQF_SHARED,
- mmc_hostname(host-mmc), host);
+ /* try to enable MSI */
+ ret = sdhci_try_enable_msi(host);
if (ret)
- return ret;
+ pr_warn(%s: Fall back to Legacy Pin-based Interrupt:
%d\n,
+ mmc_hostname(host-mmc), ret);
+
+ if (!host-msi_enabled) {
+ /* fall back to legacy pin-based interrupt */
+ ret = request_irq(host-irq, sdhci_irq, IRQF_SHARED,
+ mmc_hostname(host-mmc), host);
+ if (ret) {
+ pr_err(%s: Failed to request IRQ %d: %d\n,
+ mmc_hostname(host-mmc), host-irq,
ret);
+ return ret;
+ }
+ }
} else {
sdhci_disable_irq_wakeups(host);
disable_irq_wake(host-irq);
@@
Re: [RFC PATCH 4/4] sdio: pm: set device's power state after driver runtime suspended it
On 10/22/2012 03:57 AM, Rafael J. Wysocki wrote:
On Saturday 20 of October 2012 15:15:41 Aaron Lu wrote:
On Fri, Oct 19, 2012 at 08:08:38PM +0200, Rafael J. Wysocki wrote:
On Friday 19 of October 2012 01:39:25 Rafael J. Wysocki wrote:
On Friday 12 of October 2012 11:12:41 Aaron Lu wrote:
In sdio bus level runtime callback function, after call the driver's
runtime suspend callback, we will check if the device supports a
platform level power management, and if so, a proper power state is
chosen by the corresponding platform callback and then set.
Platform level runtime wakeup is also set, if device is enabled for
runtime wakeup by its driver, it will be armed the ability to generate
a wakeup event by the platform.
Signed-off-by: Aaron Lu aaron...@intel.com
---
drivers/mmc/core/sdio_bus.c | 49
+++--
1 file changed, 47 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index aaec9e2..d83dea8 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -23,6 +23,7 @@
#include sdio_cis.h
#include sdio_bus.h
+#include sdio.h
#include sdio_acpi.h
/* show configuration fields */
@@ -194,10 +195,54 @@ static int sdio_bus_remove(struct device *dev)
}
#ifdef CONFIG_PM
+
+static int sdio_bus_runtime_suspend(struct device *dev)
+{
+ int ret;
+ sdio_power_t state;
+
+ ret = pm_generic_runtime_suspend(dev);
+ if (ret)
+ goto out;
+
+ if (!platform_sdio_power_manageable(dev))
+ goto out;
+
+ platform_sdio_run_wake(dev, true);
+
+ state = platform_sdio_choose_power_state(dev);
+ if (state == SDIO_POWER_ERROR) {
+ ret = -EIO;
+ goto out;
+ }
+
+ ret = platform_sdio_set_power_state(dev, state);
+
+out:
+ return ret;
+}
+
+static int sdio_bus_runtime_resume(struct device *dev)
+{
+ int ret;
+
+ if (platform_sdio_power_manageable(dev)) {
+ platform_sdio_run_wake(dev, false);
+ ret = platform_sdio_set_power_state(dev, SDIO_D0);
+ if (ret)
+ goto out;
+ }
+
+ ret = pm_generic_runtime_resume(dev);
+
+out:
+ return ret;
+}
+
Most likely we will need to make analogous changes for other bus types that
don't support power management natively, like platform, SPI, I2C etc. In
all
of them the _runtime_suspend() and _runtime_resume() routine will look
almost exactly the same except for the platform_sdio_ prefix.
For this reason, I think it would be better to simply define two functions
acpi_pm_runtime_suspend() and acpi_pm_runtime_resume() that will do all of
the ACPI-specific operations related to runtime suspend/resume. Then, we
will be able to use these functions for all of the bus types in question
in the same way (we may also need to add analogous functions for system
suspend/resume handling).
Something like in the (totally untested) patch below.
Looks good to me.
I'll test the code and put it into v2 of the patchset with your
sign-off, is it OK?
I'd rather do it a bit differently in the signed-off version (I'm working
on these patches, they should be ready around Tuesday), but if you can test
OK, thanks.
it in its current form, that'd be useful too.
I was planning to test it some time later, so looks like I can directly
test your signed-off version :-)
Thanks,
Aaron
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/4] mmc: sdio: bind sdio device with acpi device
On Fri, Oct 19, 2012 at 01:25:46AM +0200, Rafael J. Wysocki wrote:
On Friday 12 of October 2012 11:12:39 Aaron Lu wrote:
ACPI spec defines the _ADDR encoding for sdio bus as:
High word - slot number (0 based)
Low word - function number
This patch adds support for sdio device binding with acpi using the
generic ACPI glue framework.
Signed-off-by: Aaron Lu aaron...@intel.com
---
drivers/mmc/core/Makefile| 1 +
drivers/mmc/core/sdio_acpi.c | 35 +++
drivers/mmc/core/sdio_bus.c | 14 --
drivers/mmc/core/sdio_bus.h | 2 ++
4 files changed, 50 insertions(+), 2 deletions(-)
create mode 100644 drivers/mmc/core/sdio_acpi.c
diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
index 38ed210..c8300aa 100644
--- a/drivers/mmc/core/Makefile
+++ b/drivers/mmc/core/Makefile
@@ -10,3 +10,4 @@ mmc_core-y:= core.o bus.o host.o \
quirks.o slot-gpio.o
mmc_core-$(CONFIG_DEBUG_FS)+= debugfs.o
+mmc_core-$(CONFIG_ACPI)+= sdio_acpi.o
diff --git a/drivers/mmc/core/sdio_acpi.c b/drivers/mmc/core/sdio_acpi.c
new file mode 100644
index 000..0f92e90
--- /dev/null
+++ b/drivers/mmc/core/sdio_acpi.c
@@ -0,0 +1,35 @@
+#include linux/mmc/host.h
+#include linux/mmc/card.h
+#include linux/mmc/sdhci.h
+#include linux/mmc/sdio_func.h
+#include linux/acpi.h
+#include acpi/acpi_bus.h
+#include sdio_bus.h
+
+static int acpi_sdio_find_device(struct device *dev, acpi_handle *handle)
+{
+ struct sdio_func *func;
+ struct sdhci_host *host;
+ u64 addr;
+
+ func = dev_to_sdio_func(dev);
+ host = mmc_priv(func-card-host);
+
+ addr = (host-slotno 16) | func-num;
+
+ *handle = acpi_get_child(DEVICE_ACPI_HANDLE(dev), addr);
+ if (!*handle)
+ return -ENODEV;
+
+ return 0;
+}
+
+static struct acpi_bus_type acpi_sdio_bus = {
+ .bus = sdio_bus_type,
+ .find_device = acpi_sdio_find_device,
+};
+
+int sdio_acpi_register(void)
I'd call it sdio_acpi_register_bus().
OK, will update this in v2.
+{
+ return register_acpi_bus_type(acpi_sdio_bus);
+}
diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index 6bf6879..aaec9e2 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -23,6 +23,7 @@
#include sdio_cis.h
#include sdio_bus.h
+#include sdio_acpi.h
/* show configuration fields */
#define sdio_config_attr(field, format_string)
\
@@ -209,7 +210,7 @@ static const struct dev_pm_ops sdio_bus_pm_ops = {
#endif /* !CONFIG_PM */
-static struct bus_type sdio_bus_type = {
+struct bus_type sdio_bus_type = {
.name = sdio,
.dev_attrs = sdio_dev_attrs,
.match = sdio_bus_match,
@@ -221,7 +222,16 @@ static struct bus_type sdio_bus_type = {
int sdio_register_bus(void)
{
- return bus_register(sdio_bus_type);
+ int ret;
+
+ ret = bus_register(sdio_bus_type);
+ if (ret)
+ goto out;
+
+ ret = sdio_acpi_register();
What happens if this fails?
Oh, yes, it shouldn't affect the sdio_register_bus function. So I'll not
check its return value in v2.
Thanks,
Aaron
+
+out:
+ return ret;
}
void sdio_unregister_bus(void)
diff --git a/drivers/mmc/core/sdio_bus.h b/drivers/mmc/core/sdio_bus.h
index 567a768..91c0e93 100644
--- a/drivers/mmc/core/sdio_bus.h
+++ b/drivers/mmc/core/sdio_bus.h
@@ -18,5 +18,7 @@ void sdio_remove_func(struct sdio_func *func);
int sdio_register_bus(void);
void sdio_unregister_bus(void);
+extern struct bus_type sdio_bus_type;
+
#endif
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 4/4] sdio: pm: set device's power state after driver runtime suspended it
On Fri, Oct 19, 2012 at 08:08:38PM +0200, Rafael J. Wysocki wrote:
On Friday 19 of October 2012 01:39:25 Rafael J. Wysocki wrote:
On Friday 12 of October 2012 11:12:41 Aaron Lu wrote:
In sdio bus level runtime callback function, after call the driver's
runtime suspend callback, we will check if the device supports a
platform level power management, and if so, a proper power state is
chosen by the corresponding platform callback and then set.
Platform level runtime wakeup is also set, if device is enabled for
runtime wakeup by its driver, it will be armed the ability to generate
a wakeup event by the platform.
Signed-off-by: Aaron Lu aaron...@intel.com
---
drivers/mmc/core/sdio_bus.c | 49
+++--
1 file changed, 47 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index aaec9e2..d83dea8 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -23,6 +23,7 @@
#include sdio_cis.h
#include sdio_bus.h
+#include sdio.h
#include sdio_acpi.h
/* show configuration fields */
@@ -194,10 +195,54 @@ static int sdio_bus_remove(struct device *dev)
}
#ifdef CONFIG_PM
+
+static int sdio_bus_runtime_suspend(struct device *dev)
+{
+ int ret;
+ sdio_power_t state;
+
+ ret = pm_generic_runtime_suspend(dev);
+ if (ret)
+ goto out;
+
+ if (!platform_sdio_power_manageable(dev))
+ goto out;
+
+ platform_sdio_run_wake(dev, true);
+
+ state = platform_sdio_choose_power_state(dev);
+ if (state == SDIO_POWER_ERROR) {
+ ret = -EIO;
+ goto out;
+ }
+
+ ret = platform_sdio_set_power_state(dev, state);
+
+out:
+ return ret;
+}
+
+static int sdio_bus_runtime_resume(struct device *dev)
+{
+ int ret;
+
+ if (platform_sdio_power_manageable(dev)) {
+ platform_sdio_run_wake(dev, false);
+ ret = platform_sdio_set_power_state(dev, SDIO_D0);
+ if (ret)
+ goto out;
+ }
+
+ ret = pm_generic_runtime_resume(dev);
+
+out:
+ return ret;
+}
+
Most likely we will need to make analogous changes for other bus types that
don't support power management natively, like platform, SPI, I2C etc. In
all
of them the _runtime_suspend() and _runtime_resume() routine will look
almost exactly the same except for the platform_sdio_ prefix.
For this reason, I think it would be better to simply define two functions
acpi_pm_runtime_suspend() and acpi_pm_runtime_resume() that will do all of
the ACPI-specific operations related to runtime suspend/resume. Then, we
will be able to use these functions for all of the bus types in question
in the same way (we may also need to add analogous functions for system
suspend/resume handling).
Something like in the (totally untested) patch below.
Looks good to me.
I'll test the code and put it into v2 of the patchset with your
sign-off, is it OK?
Thanks,
Aaron
With this code the SDIO bus type should be able to use
acpi_subsys_runtime_suspend() and acpi_subsys_runtime_resume() as its
bus-type-level runtime suspend/resume callback routines, respectively.
Thanks,
Rafael
Prototype, no sign-off.
---
drivers/acpi/dev_pm.c | 54
++
include/linux/acpi.h | 12 +++
2 files changed, 66 insertions(+)
Index: linux-pm/drivers/acpi/dev_pm.c
===
--- /dev/null
+++ linux-pm/drivers/acpi/dev_pm.c
@@ -0,0 +1,54 @@
+#include linux/acpi.h
+#include linux/device.h
+#include linux/kernel.h
+#include linux/pm_qos.h
+#include linux/pm_runtime.h
+
+#include acpi/acpi_bus.h
+
+int acpi_dev_runtime_suspend(struct device *dev)
+{
+ acpi_handle handle = DEVICE_ACPI_HANDLE(dev);
+ bool enable_wakeup;
+ int power_state;
+
+ if (!handle || !acpi_bus_power_manageable(handle))
+ return 0;
+
+ enable_wakeup = dev_pm_qos_flags(dev, PM_QOS_FLAG_REMOTE_WAKEUP))
+ != PM_QOS_FLAGS_NONE;
+ acpi_pm_device_run_wake(dev, enable_wakeup);
+
+ power_state = acpi_pm_device_sleep_state(dev, NULL, ACPI_STATE_D3);
+ if (power_state ACPI_STATE_D0 || power_state ACPI_STATE_D3)
+ return -EIO;
+
+ return acpi_bus_set_power(handle, power_state);
+}
+EXPORT_SYMBOL_GPL(acpi_dev_runtime_suspend);
+
+int acpi_dev_runtime_resume(struct device *dev)
+{
+ acpi_handle handle = DEVICE_ACPI_HANDLE(dev);
+
+ if (!handle || !acpi_bus_power_manageable(handle))
+ return 0;
+
+ acpi_pm_device_run_wake(dev, false);
+ return acpi_bus_set_power(handle, ACPI_STATE_D0);
+}
+EXPORT_SYMBOL_GPL(acpi_dev_runtime_resume);
+
+int acpi_subsys_runtime_suspend
[RFC PATCH 0/4] Enable setting of sdio device power state with ACPI
This patchset enables setting of acpi power state for sdio device when it is runtime suspended/resumed. SDIO function drivers can use the interfaces too introduced in patch 3 when there is such a need. Patch 1 adds slot number information to the sdhci_host structure, the slot number will be used to bind the sdio device with acpi node. Patch 2 does the actual binding. Patch 3 introduces a platform power management operation structure, in which 4 callbacks are defined. The reason this structure is used is that in addition to ACPI, there may be other platform mechanisms to place a device into a low power state. And a implementation of this structure is done for ACPI in this patch. The idea and the definition of this structure is heavily based on the one used in PCI subsystem. Patch 4 utilizes the newly added platform pm api to set the device into a low power state after its driver's runtime suspend callback. Its runtime wakeup capability is also enabled, and for this to work, driver must call device_set_run_wake(dev, true) somewhere when wakeup is desired. Please note that this wakeup mentioned here is realized through a sideband signal, it's not done by a SDIO interrupt. So platform support code is needed. Aaron Lu (4): sdhci: add slot number into sdhci_host structure mmc: sdio: bind sdio device with acpi device sdio: introduce sdio_platform_pm_ops sdio: pm: set device's power state after driver runtime suspended it drivers/mmc/core/Makefile | 1 + drivers/mmc/core/sdio.c| 37 +++ drivers/mmc/core/sdio.h| 28 +++ drivers/mmc/core/sdio_acpi.c | 81 ++ drivers/mmc/core/sdio_bus.c| 63 +--- drivers/mmc/core/sdio_bus.h| 2 ++ drivers/mmc/host/sdhci-pci.c | 2 +- drivers/mmc/host/sdhci-pltfm.c | 4 +-- drivers/mmc/host/sdhci-s3c.c | 2 +- drivers/mmc/host/sdhci-spear.c | 4 +-- drivers/mmc/host/sdhci.c | 3 +- drivers/mmc/host/sdhci.h | 2 +- include/linux/mmc/sdhci.h | 1 + 13 files changed, 218 insertions(+), 12 deletions(-) create mode 100644 drivers/mmc/core/sdio.h create mode 100644 drivers/mmc/core/sdio_acpi.c -- 1.7.12.21.g871e293 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 2/4] mmc: sdio: bind sdio device with acpi device
ACPI spec defines the _ADDR encoding for sdio bus as:
High word - slot number (0 based)
Low word - function number
This patch adds support for sdio device binding with acpi using the
generic ACPI glue framework.
Signed-off-by: Aaron Lu aaron...@intel.com
---
drivers/mmc/core/Makefile| 1 +
drivers/mmc/core/sdio_acpi.c | 35 +++
drivers/mmc/core/sdio_bus.c | 14 --
drivers/mmc/core/sdio_bus.h | 2 ++
4 files changed, 50 insertions(+), 2 deletions(-)
create mode 100644 drivers/mmc/core/sdio_acpi.c
diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
index 38ed210..c8300aa 100644
--- a/drivers/mmc/core/Makefile
+++ b/drivers/mmc/core/Makefile
@@ -10,3 +10,4 @@ mmc_core-y:= core.o bus.o host.o \
quirks.o slot-gpio.o
mmc_core-$(CONFIG_DEBUG_FS)+= debugfs.o
+mmc_core-$(CONFIG_ACPI)+= sdio_acpi.o
diff --git a/drivers/mmc/core/sdio_acpi.c b/drivers/mmc/core/sdio_acpi.c
new file mode 100644
index 000..0f92e90
--- /dev/null
+++ b/drivers/mmc/core/sdio_acpi.c
@@ -0,0 +1,35 @@
+#include linux/mmc/host.h
+#include linux/mmc/card.h
+#include linux/mmc/sdhci.h
+#include linux/mmc/sdio_func.h
+#include linux/acpi.h
+#include acpi/acpi_bus.h
+#include sdio_bus.h
+
+static int acpi_sdio_find_device(struct device *dev, acpi_handle *handle)
+{
+ struct sdio_func *func;
+ struct sdhci_host *host;
+ u64 addr;
+
+ func = dev_to_sdio_func(dev);
+ host = mmc_priv(func-card-host);
+
+ addr = (host-slotno 16) | func-num;
+
+ *handle = acpi_get_child(DEVICE_ACPI_HANDLE(dev), addr);
+ if (!*handle)
+ return -ENODEV;
+
+ return 0;
+}
+
+static struct acpi_bus_type acpi_sdio_bus = {
+ .bus = sdio_bus_type,
+ .find_device = acpi_sdio_find_device,
+};
+
+int sdio_acpi_register(void)
+{
+ return register_acpi_bus_type(acpi_sdio_bus);
+}
diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index 6bf6879..aaec9e2 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -23,6 +23,7 @@
#include sdio_cis.h
#include sdio_bus.h
+#include sdio_acpi.h
/* show configuration fields */
#define sdio_config_attr(field, format_string) \
@@ -209,7 +210,7 @@ static const struct dev_pm_ops sdio_bus_pm_ops = {
#endif /* !CONFIG_PM */
-static struct bus_type sdio_bus_type = {
+struct bus_type sdio_bus_type = {
.name = sdio,
.dev_attrs = sdio_dev_attrs,
.match = sdio_bus_match,
@@ -221,7 +222,16 @@ static struct bus_type sdio_bus_type = {
int sdio_register_bus(void)
{
- return bus_register(sdio_bus_type);
+ int ret;
+
+ ret = bus_register(sdio_bus_type);
+ if (ret)
+ goto out;
+
+ ret = sdio_acpi_register();
+
+out:
+ return ret;
}
void sdio_unregister_bus(void)
diff --git a/drivers/mmc/core/sdio_bus.h b/drivers/mmc/core/sdio_bus.h
index 567a768..91c0e93 100644
--- a/drivers/mmc/core/sdio_bus.h
+++ b/drivers/mmc/core/sdio_bus.h
@@ -18,5 +18,7 @@ void sdio_remove_func(struct sdio_func *func);
int sdio_register_bus(void);
void sdio_unregister_bus(void);
+extern struct bus_type sdio_bus_type;
+
#endif
--
1.7.12.21.g871e293
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 3/4] sdio: introduce sdio_platform_pm_ops
Some platform has the ability to set the sdio device into a low power
state with some specific method, e.g. ACPI on x86 based system can use
acpi control methods to change the device's power state.
Considering there may be different platforms utilizing different
mechanisms to achieve this, a new structure is introduced to let
individual platform to use these callbacks to do the job.
The structure contains 4 callbacks:
- is_manageable
return true when the platform can manage the device's power;
return false otherwise.
- choose_state
Choose a proper power state for the device
- set_state
Set the device's power state
- run_wake
Enable the device's runtime wakeup capability from the platform's
perspective.
And 4 functions to wrap these callbacks:
- bool platform_sdio_power_manageable(struct device *dev)
- int platform_sdio_choose_power_state(struct device *dev)
- int platform_sdio_set_power_state(struct device *dev, int state)
- int platform_sdio_run_wake(struct device *dev, bool enable)
So when these callbacks are desired, these wrapper functions should be
used. And if someday some sdio function driver which lives out of the
mmc subsystem has a need to use these wrapper functions, they can be
exported.
sdio_acpi.c implements these callbacks utilizing ACPI code.
The idea of this patch and the definition/wrapper of these callbacks
are heavily based on the one used in PCI subsystem.
Signed-off-by: Aaron Lu aaron...@intel.com
---
drivers/mmc/core/sdio.c | 37 ++
drivers/mmc/core/sdio.h | 28 ++
drivers/mmc/core/sdio_acpi.c | 48 +++-
3 files changed, 112 insertions(+), 1 deletion(-)
create mode 100644 drivers/mmc/core/sdio.h
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index d4619e2..84b01b2 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -27,6 +27,7 @@
#include sd_ops.h
#include sdio_ops.h
#include sdio_cis.h
+#include sdio.h
static int sdio_read_fbr(struct sdio_func *func)
{
@@ -1178,3 +1179,39 @@ err:
return err;
}
+static struct sdio_platform_pm_ops *sdio_platform_pm;
+
+int sdio_set_platform_pm(struct sdio_platform_pm_ops *ops)
+{
+ if (!ops-is_manageable || !ops-choose_state ||
+ !ops-set_state || !ops-run_wake)
+ return -EINVAL;
+
+ sdio_platform_pm = ops;
+
+ return 0;
+}
+
+bool platform_sdio_power_manageable(struct device *dev)
+{
+ return sdio_platform_pm ?
+ sdio_platform_pm-is_manageable(dev) : false;
+}
+
+int platform_sdio_run_wake(struct device *dev, bool enable)
+{
+ return sdio_platform_pm ?
+ sdio_platform_pm-run_wake(dev, enable) : -ENODEV;
+}
+
+int platform_sdio_choose_power_state(struct device *dev)
+{
+ return sdio_platform_pm ?
+ sdio_platform_pm-choose_state(dev) : SDIO_POWER_ERROR;
+}
+
+int platform_sdio_set_power_state(struct device *dev, int state)
+{
+ return sdio_platform_pm ?
+ sdio_platform_pm-set_state(dev, state) : -ENOSYS;
+}
diff --git a/drivers/mmc/core/sdio.h b/drivers/mmc/core/sdio.h
new file mode 100644
index 000..a95929e
--- /dev/null
+++ b/drivers/mmc/core/sdio.h
@@ -0,0 +1,28 @@
+#ifndef __SDIO_H
+#define __SDIO_H
+
+typedef int __bitwise sdio_power_t;
+
+#define SDIO_D0 ((sdio_power_t __force) 0)
+#define SDIO_D1 ((sdio_power_t __force) 1)
+#define SDIO_D2 ((sdio_power_t __force) 2)
+#define SDIO_D3hot ((sdio_power_t __force) 3)
+#define SDIO_D3cold ((sdio_power_t __force) 4)
+#define SDIO_UNKNOWN ((sdio_power_t __force) 5)
+#define SDIO_POWER_ERROR ((sdio_power_t __force) -1)
+
+struct sdio_platform_pm_ops {
+ bool (*is_manageable)(struct device *dev);
+ int (*choose_state)(struct device *dev);
+ int (*set_state)(struct device *dev, sdio_power_t state);
+ int (*run_wake)(struct device *dev, bool enabel);
+};
+
+int sdio_set_platform_pm(struct sdio_platform_pm_ops *ops);
+
+bool platform_sdio_power_manageable(struct device *dev);
+sdio_power_t platform_sdio_choose_power_state(struct device *dev);
+int platform_sdio_set_power_state(struct device *dev, sdio_power_t state);
+int platform_sdio_run_wake(struct device *dev, bool enable);
+
+#endif
diff --git a/drivers/mmc/core/sdio_acpi.c b/drivers/mmc/core/sdio_acpi.c
index 0f92e90..a5b3012 100644
--- a/drivers/mmc/core/sdio_acpi.c
+++ b/drivers/mmc/core/sdio_acpi.c
@@ -4,8 +4,45 @@
#include linux/mmc/sdio_func.h
#include linux/acpi.h
#include acpi/acpi_bus.h
+#include sdio.h
#include sdio_bus.h
+static bool acpi_sdio_power_manageable(struct device *dev)
+{
+ acpi_handle handle = DEVICE_ACPI_HANDLE(dev);
+ return handle ? acpi_bus_power_manageable(handle) : false;
+}
+
+static int acpi_sdio_choose_power_state(struct device *dev)
+{
+ return acpi_pm_device_sleep_state(dev, NULL, ACPI_STATE_D3);
+}
+
+static int
[RFC PATCH 4/4] sdio: pm: set device's power state after driver runtime suspended it
In sdio bus level runtime callback function, after call the driver's
runtime suspend callback, we will check if the device supports a
platform level power management, and if so, a proper power state is
chosen by the corresponding platform callback and then set.
Platform level runtime wakeup is also set, if device is enabled for
runtime wakeup by its driver, it will be armed the ability to generate
a wakeup event by the platform.
Signed-off-by: Aaron Lu aaron...@intel.com
---
drivers/mmc/core/sdio_bus.c | 49 +++--
1 file changed, 47 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index aaec9e2..d83dea8 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -23,6 +23,7 @@
#include sdio_cis.h
#include sdio_bus.h
+#include sdio.h
#include sdio_acpi.h
/* show configuration fields */
@@ -194,10 +195,54 @@ static int sdio_bus_remove(struct device *dev)
}
#ifdef CONFIG_PM
+
+static int sdio_bus_runtime_suspend(struct device *dev)
+{
+ int ret;
+ sdio_power_t state;
+
+ ret = pm_generic_runtime_suspend(dev);
+ if (ret)
+ goto out;
+
+ if (!platform_sdio_power_manageable(dev))
+ goto out;
+
+ platform_sdio_run_wake(dev, true);
+
+ state = platform_sdio_choose_power_state(dev);
+ if (state == SDIO_POWER_ERROR) {
+ ret = -EIO;
+ goto out;
+ }
+
+ ret = platform_sdio_set_power_state(dev, state);
+
+out:
+ return ret;
+}
+
+static int sdio_bus_runtime_resume(struct device *dev)
+{
+ int ret;
+
+ if (platform_sdio_power_manageable(dev)) {
+ platform_sdio_run_wake(dev, false);
+ ret = platform_sdio_set_power_state(dev, SDIO_D0);
+ if (ret)
+ goto out;
+ }
+
+ ret = pm_generic_runtime_resume(dev);
+
+out:
+ return ret;
+}
+
static const struct dev_pm_ops sdio_bus_pm_ops = {
SET_RUNTIME_PM_OPS(
- pm_generic_runtime_suspend,
- pm_generic_runtime_resume,
+ sdio_bus_runtime_suspend,
+ sdio_bus_runtime_resume,
pm_generic_runtime_idle
)
};
--
1.7.12.21.g871e293
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: v3.6-rc1: modprobe hangs with sdhci failure on dell e6410
On Wed, Aug 22, 2012 at 10:11 PM, Arend van Spriel ar...@broadcom.com wrote: A quick search using google did not provide clues. Regardless if there is anything inserted the hang occurs. Gr. AvS Also, your .config might be helpful. Thanks, Aaron -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: v3.6-rc1: modprobe hangs with sdhci failure on dell e6410
On 08/22/2012 10:11 PM, Arend van Spriel wrote:
A quick search using google did not provide clues. Regardless if there
is anything inserted the hang occurs.
your dmesg shows:
[ 241.908294] INFO: task modprobe:134 blocked for more than 120 seconds.
[ 241.908298] echo 0 /proc/sys/kernel/hung_task_timeout_secs disables this
message.
[ 241.908301] modprobeD f48abbcc 0 134 98 0x
[ 241.908307] f48abc3c 0082 f59d0ebc f48abbcc c1289228 f4f3e480
c17b0380
[ 241.908313] c17b0380 c17b0380 1de51926 c17b0380 f59d5380 f4f3e480
f4853ed0
[ 241.908319] 0001 f48abc28 f59d0eb0 f4df3131 c17abe80
0003
[ 241.908326] Call Trace:
[ 241.908337] [c1289228] ? timerqueue_add+0x58/0xb0
[ 241.908345] [c14ca103] schedule+0x23/0x60
[ 241.908349] [c14c8f5f] schedule_hrtimeout_range_clock+0xaf/0x130
[ 241.908354] [c105b4f0] ? update_rmtp+0x80/0x80
[ 241.908359] [c105c796] ? hrtimer_start_range_ns+0x26/0x30
[ 241.908362] [c14c8ff7] schedule_hrtimeout_range+0x17/0x20
[ 241.908369] [c10462c9] usleep_range+0x39/0x40
[ 241.908384] [f8030b39] sdhci_do_start_signal_voltage_switch+0x59/0x150
[sdhci]
[ 241.908390] [f8030c71] sdhci_start_signal_voltage_switch+0x41/0x80 [sdhci]
[ 241.908401] [f805ded8] mmc_set_signal_voltage+0x58/0xb0 [mmc_core]
[ 241.908411] [f805e105] mmc_power_up+0x85/0xf0 [mmc_core]
[ 241.908420] [f805e1a8] mmc_start_host+0x38/0x50 [mmc_core]
[ 241.908430] [f805f4b0] mmc_add_host+0x50/0x90 [mmc_core]
[ 241.908436] [f8031b37] sdhci_add_host+0x837/0xbc0 [sdhci]
[ 241.908444] [f807f10a] sdhci_pci_probe+0x3fc/0x5f0 [sdhci_pci]
[ 241.908449] [c14cac5f] ? _raw_spin_lock_irqsave+0x2f/0x50
[ 241.908455] [c12a5407] local_pci_probe+0x47/0xb0
[ 241.908460] [c12a6458] pci_device_probe+0x68/0x90
[ 241.908467] [c1337c48] driver_probe_device+0x78/0x1f0
[ 241.908471] [c12a5393] ? pci_match_device+0xb3/0xc0
[ 241.908476] [c1337e41] __driver_attach+0x81/0x90
[ 241.908480] [c1336613] bus_for_each_dev+0x53/0x80
[ 241.908484] [c1337abe] driver_attach+0x1e/0x20
[ 241.908488] [c1337dc0] ? driver_probe_device+0x1f0/0x1f0
[ 241.908491] [c1337512] bus_add_driver+0xb2/0x230
[ 241.908495] [c12a62d0] ? pci_dev_put+0x20/0x20
[ 241.908499] [c12a62d0] ? pci_dev_put+0x20/0x20
[ 241.908502] [c133841a] driver_register+0x6a/0x140
[ 241.908509] [c10be41b] ? tracepoint_module_notify+0x12b/0x190
[ 241.908514] [c12a6694] __pci_register_driver+0x44/0xb0
[ 241.908522] [f8086017] sdhci_drv_init+0x17/0x19 [sdhci_pci]
[ 241.908526] [c1001114] do_one_initcall+0x34/0x170
[ 241.908532] [f8086000] ? 0xf8085fff
[ 241.908539] [c1090c3e] sys_init_module+0xee/0x1460
[ 241.908542] [c108f620] ? free_notes_attrs+0x50/0x50
[ 241.908549] [f8073000] ? 0xf8072fff
[ 241.908556] [c14d175f] sysenter_do_call+0x12/0x28
Looks like the usleep_range called in
sdhci_do_start_signal_voltage_switch blocked modprobe, maybe the timeout
never happens for whatever reason?
Can you please try the following patch:
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 8ac5246..30ce05d 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1212,6 +1212,9 @@ static void mmc_power_up(struct mmc_host *host)
host-ios.timing = MMC_TIMING_LEGACY;
mmc_set_ios(host);
+ /* debug */
+ usleep_range(5000, 5500);
+
/* Set signal voltage to 3.3V */
mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330, false);
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 9a11dc3..a181c46 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1616,7 +1616,8 @@ static int sdhci_do_start_signal_voltage_switch(struct
sdhci_host *host,
* to 3.3V. If so, we change the voltage to 3.3V and return quickly.
*/
ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
- if (ios-signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
+ if (ios-signal_voltage == MMC_SIGNAL_VOLTAGE_330
+ (ctrl SDHCI_CTRL_VDD_180)) {
/* Set 1.8V Signal Enable in the Host Control2 register to 0 */
ctrl = ~SDHCI_CTRL_VDD_180;
sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
The above code did 2 things:
1 calling usleep_range in another place to see what happened;
2 avoid setting 3.3v signalling voltage if host is already at 3.3v
signalling voltage.
Thanks,
Aaron
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add MSI support for SDHCI PCI hosts
Hello,
On 08/18/2012 01:06 PM, Daniel J Blueman wrote:
Hi Philip,
The interrupt could be from another source, though Message Signalled
Interrupts are a PCI (and variants) only feature at present. I'm happy
to split the patch and/or make the module param specific to sdhci-pci.
Not sure if you are aware of the following thread:
http://marc.info/?l=linux-mmcm=133346937412708w=2
Enabling msi will break some systems.
And more comments below.
Let's see what the maintainers prefer...Chris?
Dan
On 18 August 2012 00:35, Philip Rakity prak...@marvell.com wrote:
The interrupt source could be from a non-pci device. Say a gpio or whatever.
Does it make sense to break this patch into two patches ?
On Aug 17, 2012, at 2:14 AM, Daniel J Blueman dan...@quora.org wrote:
Allow module parameter 'enable_msi' to request an MSI interrupt for
hosts where available (presently PCI). Useful as a workaround on
platforms where the legacy interrupt is broken.
Signed-off-by: Daniel J Blueman dan...@quora.org
---
drivers/mmc/host/sdhci-pci.c | 30 ++
drivers/mmc/host/sdhci.c | 23 +++
drivers/mmc/host/sdhci.h |2 ++
3 files changed, 55 insertions(+)
diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
index 504da71..fbde589 100644
--- a/drivers/mmc/host/sdhci-pci.c
+++ b/drivers/mmc/host/sdhci-pci.c
@@ -934,6 +934,34 @@ static int sdhci_pci_enable_dma(struct sdhci_host
*host)
return 0;
}
+static int sdhci_pci_enable_msi(struct sdhci_host *host)
+{
+ struct sdhci_pci_slot *slot;
+ struct pci_dev *pdev;
+ int ret;
+
+ slot = sdhci_priv(host);
+ pdev = slot-chip-pdev;
+
+ ret = pci_enable_msi(pdev);
+ if (ret)
+ return ret;
+
+ host-irq = pdev-irq;
+ return 0;
+}
+
+static void sdhci_pci_disable_msi(struct sdhci_host *host)
+{
+ struct sdhci_pci_slot *slot;
+ struct pci_dev *pdev;
+
+ slot = sdhci_priv(host);
+ pdev = slot-chip-pdev;
+
+ pci_disable_msi(pdev);
+}
+
static int sdhci_pci_8bit_width(struct sdhci_host *host, int width)
{
u8 ctrl;
@@ -976,6 +1004,8 @@ static void sdhci_pci_hw_reset(struct sdhci_host *host)
static struct sdhci_ops sdhci_pci_ops = {
.enable_dma = sdhci_pci_enable_dma,
+ .enable_msi = sdhci_pci_enable_msi,
+ .disable_msi= sdhci_pci_disable_msi,
.platform_8bit_width= sdhci_pci_8bit_width,
.hw_reset = sdhci_pci_hw_reset,
};
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 9a11dc3..9fa2180 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -45,6 +45,7 @@
static unsigned int debug_quirks = 0;
static unsigned int debug_quirks2;
+static bool enable_msi;
static void sdhci_finish_data(struct sdhci_host *);
@@ -2433,6 +2434,9 @@ int sdhci_suspend_host(struct sdhci_host *host)
free_irq(host-irq, host);
+ if (host-ops-disable_msi enable_msi)
+ host-ops-disable_msi(host);
+
Don't know why msi needs to be disabled/enabled on suspend/resume.
And doing this in sdhci_pci_suspend/resume might be more appropriate if
required.
Thanks,
Aaron
return ret;
}
@@ -2447,6 +2451,12 @@ int sdhci_resume_host(struct sdhci_host *host)
host-ops-enable_dma(host);
}
+ if (host-ops-enable_msi enable_msi) {
+ ret = host-ops-enable_msi(host);
+ if (ret)
+ return ret;
+ }
+
ret = request_irq(host-irq, sdhci_irq, IRQF_SHARED,
mmc_hostname(host-mmc), host);
if (ret)
@@ -3024,6 +3034,12 @@ int sdhci_add_host(struct sdhci_host *host)
host-tuning_timer.function = sdhci_tuning_timer;
}
+ if (host-ops-enable_msi enable_msi) {
+ ret = host-ops-enable_msi(host);
+ if (ret)
+ return ret;
+ }
+
ret = request_irq(host-irq, sdhci_irq, IRQF_SHARED,
mmc_hostname(mmc), host);
if (ret) {
@@ -3071,6 +3087,8 @@ int sdhci_add_host(struct sdhci_host *host)
reset:
sdhci_reset(host, SDHCI_RESET_ALL);
free_irq(host-irq, host);
+ if (host-ops-disable_msi enable_msi)
+ host-ops-disable_msi(host);
#endif
untasklet:
tasklet_kill(host-card_tasklet);
@@ -3114,6 +3132,9 @@ void sdhci_remove_host(struct sdhci_host *host, int
dead)
free_irq(host-irq, host);
+ if (host-ops-disable_msi enable_msi)
+ host-ops-disable_msi(host);
+
del_timer_sync(host-timer);
tasklet_kill(host-card_tasklet);
@@ -3162,6 +3183,7 @@ module_exit(sdhci_drv_exit);
module_param(debug_quirks, uint, 0444);
module_param(debug_quirks2, uint, 0444);
+module_param(enable_msi, bool, 0444);
MODULE_AUTHOR(Pierre Ossman pie...@ossman.eu);
MODULE_DESCRIPTION(Secure Digital Host Controller
Re: [PATCH 2/2] mmc: sd: Fix sd current limit setting
Hi Chris,
On Wed, Jul 18, 2012 at 01:28:40AM -0400, Chris Ball wrote:
Hi Aaron,
On Wed, Jul 18 2012, Aaron Lu wrote:
Is the following patch OK? This is based on top of current mmc-next with
the previous one in tree. Not sure if this is what you want though.
Yes, that's perfect; squashed into the original patch and pushed out
to mmc-next. Thanks!
Having there be so many MAX_CURRENT defines -- and having them be
split in the middle between CAP_ and CAP2_ -- is starting to feel
a bit awkward.
I agree.
Does anyone have ideas on how that might be tidied up,
since we have an opportunity to come up with a better plan
before this gets merged soon?
What about we add three fields in mmc_host to store the max current
value for 3.3v/3.0v/1.8v and use that when needed instead of the cap
setting of the host?
I've prepared the following code, please check if this is better than
the current one:
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 312b78d..2232004 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -517,11 +517,36 @@ static int sd_set_bus_speed_mode(struct mmc_card *card,
u8 *status)
return 0;
}
+/* Get host's max current setting at its current voltage */
+static u32 sd_get_host_max_current(struct mmc_host *host)
+{
+ u32 voltage, max_current;
+
+ voltage = 1 host-ios.vdd;
+ switch (voltage) {
+ case MMC_VDD_165_195:
+ max_current = host-max_current_180;
+ break;
+ case MMC_VDD_29_30:
+ case MMC_VDD_30_31:
+ max_current = host-max_current_300;
+ break;
+ case MMC_VDD_32_33:
+ case MMC_VDD_33_34:
+ max_current = host-max_current_330;
+ break;
+ default:
+ max_current = 0;
+ }
+
+ return max_current;
+}
+
static int sd_set_current_limit(struct mmc_card *card, u8 *status)
{
int current_limit = SD_SET_CURRENT_NO_CHANGE;
int err;
- u32 voltage;
+ u32 max_current;
/*
* Current limit switch is only defined for SDR50, SDR104, and DDR50
@@ -535,9 +560,9 @@ static int sd_set_current_limit(struct mmc_card *card, u8
*status)
/*
* Host has different current capabilities when operating at
-* different voltages, so find out the current voltage first.
+* different voltages, so find out its max current first.
*/
- voltage = 1 card-host-ios.vdd;
+ max_current = sd_get_host_max_current(card-host);
/*
* We only check host's capability here, if we set a limit that is
@@ -547,34 +572,15 @@ static int sd_set_current_limit(struct mmc_card *card, u8
*status)
* when we set current limit to 400/600/800ma, the card will draw its
* maximum 300ma from the host.
*/
- if (voltage == MMC_VDD_165_195) {
- if (card-host-caps MMC_CAP_MAX_CURRENT_800_180)
- current_limit = SD_SET_CURRENT_LIMIT_800;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_600_180)
- current_limit = SD_SET_CURRENT_LIMIT_600;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_400_180)
- current_limit = SD_SET_CURRENT_LIMIT_400;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_200_180)
- current_limit = SD_SET_CURRENT_LIMIT_200;
- } else if (voltage (MMC_VDD_29_30 | MMC_VDD_30_31)) {
- if (card-host-caps MMC_CAP_MAX_CURRENT_800_300)
- current_limit = SD_SET_CURRENT_LIMIT_800;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_600_300)
- current_limit = SD_SET_CURRENT_LIMIT_600;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_400_300)
- current_limit = SD_SET_CURRENT_LIMIT_400;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_200_300)
- current_limit = SD_SET_CURRENT_LIMIT_200;
- } else if (voltage (MMC_VDD_32_33 | MMC_VDD_33_34)) {
- if (card-host-caps MMC_CAP_MAX_CURRENT_800_330)
- current_limit = SD_SET_CURRENT_LIMIT_800;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_600_330)
- current_limit = SD_SET_CURRENT_LIMIT_600;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_400_330)
- current_limit = SD_SET_CURRENT_LIMIT_400;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_200_330)
- current_limit = SD_SET_CURRENT_LIMIT_200;
- }
+
+ if (max_current = 800)
+ current_limit = SD_SET_CURRENT_LIMIT_800;
+ else if (max_current = 600)
+ current_limit = SD_SET_CURRENT_LIMIT_600;
+ else if (max_current = 400)
+ current_limit = SD_SET_CURRENT_LIMIT_400;
+ else if (max_current = 200
Re: [PATCH 2/2] mmc: sd: Fix sd current limit setting
On Tue, Jul 17, 2012 at 03:04:05PM +0530, Girish K S wrote:
@@ -261,6 +261,14 @@ struct mmc_host {
#define MMC_CAP2_HC_ERASE_SZ (1 9)/* High-capacity erase size
*/
#define MMC_CAP2_CD_ACTIVE_HIGH(1 10) /* Card-detect
signal active high */
#define MMC_CAP2_RO_ACTIVE_HIGH(1 11) /* Write-protect
signal active high */
+#define MMC_CAP_MAX_CURRENT_200_300 (1 12) /* Host max current limit
is 200mA at 3.0V */
+#define MMC_CAP_MAX_CURRENT_400_300 (1 13) /* Host max current limit
is 400mA at 3.0V */
+#define MMC_CAP_MAX_CURRENT_600_300 (1 14) /* Host max current limit
is 600mA at 3.0V */
+#define MMC_CAP_MAX_CURRENT_800_300 (1 15) /* Host max current limit
is 800mA at 3.0V */
+#define MMC_CAP_MAX_CURRENT_200_330 (1 16) /* Host max current limit
is 200mA at 3.3V */
+#define MMC_CAP_MAX_CURRENT_400_330 (1 17) /* Host max current limit
is 400mA at 3.3V */
+#define MMC_CAP_MAX_CURRENT_600_330 (1 18) /* Host max current limit
is 600mA at 3.3V */
+#define MMC_CAP_MAX_CURRENT_800_330 (1 19) /* Host max current limit
is 800mA at 3.3V */
sorry for seeing this late.
The macro name should be **_CAP2_***.
Thanks for pointing this out, I'll be more careful next time.
This is already merged in chris's mmc-next branch,
Chris,
Is it possible to change the macro name in the mmc-next branch for this patch?
Chris, please let me know how you want to deal with this and I'll be
glad to do whatever I can to fix this. Sorry for the trouble.
-Aaron
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mmc: sd: Fix sd current limit setting
Hi,
On Tue, Jul 17, 2012 at 11:43:26AM -0400, Chris Ball wrote:
Chris, please let me know how you want to deal with this and I'll be
glad to do whatever I can to fix this. Sorry for the trouble.
No worries, I can rebase it in. Mind sending a patch on top of current
mmc-next for me to squash on top of the previous one?
Is the following patch OK? This is based on top of current mmc-next with
the previous one in tree. Not sure if this is what you want though.
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 312b78d..ec03d15 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -557,22 +557,22 @@ static int sd_set_current_limit(struct mmc_card *card, u8
*status)
else if (card-host-caps MMC_CAP_MAX_CURRENT_200_180)
current_limit = SD_SET_CURRENT_LIMIT_200;
} else if (voltage (MMC_VDD_29_30 | MMC_VDD_30_31)) {
- if (card-host-caps MMC_CAP_MAX_CURRENT_800_300)
+ if (card-host-caps2 MMC_CAP2_MAX_CURRENT_800_300)
current_limit = SD_SET_CURRENT_LIMIT_800;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_600_300)
+ else if (card-host-caps2 MMC_CAP2_MAX_CURRENT_600_300)
current_limit = SD_SET_CURRENT_LIMIT_600;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_400_300)
+ else if (card-host-caps2 MMC_CAP2_MAX_CURRENT_400_300)
current_limit = SD_SET_CURRENT_LIMIT_400;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_200_300)
+ else if (card-host-caps2 MMC_CAP2_MAX_CURRENT_200_300)
current_limit = SD_SET_CURRENT_LIMIT_200;
} else if (voltage (MMC_VDD_32_33 | MMC_VDD_33_34)) {
- if (card-host-caps MMC_CAP_MAX_CURRENT_800_330)
+ if (card-host-caps2 MMC_CAP2_MAX_CURRENT_800_330)
current_limit = SD_SET_CURRENT_LIMIT_800;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_600_330)
+ else if (card-host-caps2 MMC_CAP2_MAX_CURRENT_600_330)
current_limit = SD_SET_CURRENT_LIMIT_600;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_400_330)
+ else if (card-host-caps2 MMC_CAP2_MAX_CURRENT_400_330)
current_limit = SD_SET_CURRENT_LIMIT_400;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_200_330)
+ else if (card-host-caps2 MMC_CAP2_MAX_CURRENT_200_330)
current_limit = SD_SET_CURRENT_LIMIT_200;
}
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 455a093..29d4357 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2920,13 +2920,13 @@ int sdhci_add_host(struct sdhci_host *host)
/* Maximum current capabilities of the host at 3.3V */
if (max_current_330 = 800)
- mmc-caps |= MMC_CAP_MAX_CURRENT_800_330;
+ mmc-caps2 |= MMC_CAP2_MAX_CURRENT_800_330;
else if (max_current_330 = 600)
- mmc-caps |= MMC_CAP_MAX_CURRENT_600_330;
+ mmc-caps2 |= MMC_CAP2_MAX_CURRENT_600_330;
else if (max_current_330 = 400)
- mmc-caps |= MMC_CAP_MAX_CURRENT_400_330;
+ mmc-caps2 |= MMC_CAP2_MAX_CURRENT_400_330;
else if (max_current_330 = 200)
- mmc-caps |= MMC_CAP_MAX_CURRENT_200_330;
+ mmc-caps2 |= MMC_CAP2_MAX_CURRENT_200_330;
}
if (caps[0] SDHCI_CAN_VDD_300) {
int max_current_300;
@@ -2943,13 +2943,13 @@ int sdhci_add_host(struct sdhci_host *host)
/* Maximum current capabilities of the host at 3.0V */
if (max_current_300 = 800)
- mmc-caps |= MMC_CAP_MAX_CURRENT_800_300;
+ mmc-caps2 |= MMC_CAP2_MAX_CURRENT_800_300;
else if (max_current_300 = 600)
- mmc-caps |= MMC_CAP_MAX_CURRENT_600_300;
+ mmc-caps2 |= MMC_CAP2_MAX_CURRENT_600_300;
else if (max_current_300 = 400)
- mmc-caps |= MMC_CAP_MAX_CURRENT_400_300;
+ mmc-caps2 |= MMC_CAP2_MAX_CURRENT_400_300;
else if (max_current_300 = 200)
- mmc-caps |= MMC_CAP_MAX_CURRENT_200_300;
+ mmc-caps2 |= MMC_CAP2_MAX_CURRENT_200_300;
}
if (caps[0] SDHCI_CAN_VDD_180) {
int max_current_180;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 79d8921..4a40312 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -261,14 +261,14 @@ struct mmc_host {
#define MMC_CAP2_HC_ERASE_SZ (1 9)/* High-capacity erase size */
#define MMC_CAP2_CD_ACTIVE_HIGH(1 10)
[PATCH] mmc: core: reset sigal voltage on power up
Add a call to mmc_set_signal_voltage to set signal voltage to 3.3v in
mmc_power_up so that we do not need to touch signal voltage setting in
mmc/sd/sdio init functions and rescan function.
For mmc/sd cards, when doing a suspend/resume cycle, consider the unsafe
resume case, the card will lose its power and when powered on again, we
will set signal voltage to 3.3v in mmc_power_up before its resume function
gets called, which will re-init the card.
And for sdio cards, when doing a suspend/resume cycle, consider the unsafe
resume case, the card will either lose its power or not depending on if it
wants to wakeup the host. If power is not maintained, it is the same case as
mmc/sd cards. If power is maintained, mmc_power_up will not be called and
the card's signal voltage will remain at the last setting.
Signed-off-by: Aaron Lu aaron...@amd.com
---
drivers/mmc/core/core.c | 6 +++---
drivers/mmc/core/mmc.c | 3 ---
drivers/mmc/core/sd.c | 3 ---
drivers/mmc/core/sdio.c | 7 ---
4 files changed, 3 insertions(+), 16 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 9503cab..8ac5246 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1212,6 +1212,9 @@ static void mmc_power_up(struct mmc_host *host)
host-ios.timing = MMC_TIMING_LEGACY;
mmc_set_ios(host);
+ /* Set signal voltage to 3.3V */
+ mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330, false);
+
/*
* This delay should be sufficient to allow the power supply
* to reach the minimum voltage.
@@ -1963,9 +1966,6 @@ static int mmc_rescan_try_freq(struct mmc_host *host,
unsigned freq)
*/
mmc_hw_reset_for_init(host);
- /* Initialization should be done at 3.3 V I/O voltage. */
- mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330, 0);
-
/*
* sdio_reset sends CMD52 to reset card. Since we do not know
* if the card is being re-initialized, just send it. CMD52
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 4f4489a..396b258 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -818,9 +818,6 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
if (!mmc_host_is_spi(host))
mmc_set_bus_mode(host, MMC_BUSMODE_OPENDRAIN);
- /* Initialization should be done at 3.3 V I/O voltage. */
- mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330, 0);
-
/*
* Since we're changing the OCR value, we seem to
* need to tell some cards to go back to the idle
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 312b78d..2182d92 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -892,9 +892,6 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
BUG_ON(!host);
WARN_ON(!host-claimed);
- /* The initialization should be done at 3.3 V I/O voltage. */
- mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330, 0);
-
err = mmc_sd_get_cid(host, ocr, cid, rocr);
if (err)
return err;
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 41c5fd8..d4619e2 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -591,9 +591,6 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32
ocr,
* Inform the card of the voltage
*/
if (!powered_resume) {
- /* The initialization should be done at 3.3 V I/O voltage. */
- mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330, 0);
-
err = mmc_send_io_op_cond(host, host-ocr, ocr);
if (err)
goto err;
@@ -1006,10 +1003,6 @@ static int mmc_sdio_power_restore(struct mmc_host *host)
* restore the correct voltage setting of the card.
*/
- /* The initialization should be done at 3.3 V I/O voltage. */
- if (!mmc_card_keep_power(host))
- mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330, 0);
-
sdio_reset(host);
mmc_go_idle(host);
mmc_send_if_cond(host, host-ocr_avail);
--
1.7.11.1.3.g4c8a9db
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: host: sdhci. UHS Switch failure - cycle power if regulator is used
On 06/28/2012 12:15 AM, philipspatc...@gmail.com wrote: From: Philip Rakityprak...@marvell.com Power needs to be removed from the card when switching to 1.8v fails. If a regulator is used to control vmmc we need to turn the regulator off and then back on otherwise power will not be removed from the card. Reviewed-by: Aaron Lu aaron...@amd.com -Aaron Signed-off-by: Philip Rakityprak...@marvell.com --- drivers/mmc/host/sdhci.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index f4b8b4d..ef4231a 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1663,11 +1663,15 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host, pwr = sdhci_readb(host, SDHCI_POWER_CONTROL); pwr= ~SDHCI_POWER_ON; sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL); + if (host-vmmc) + regulator_disable(host-vmmc); /* Wait for 1ms as per the spec */ usleep_range(1000, 1500); pwr |= SDHCI_POWER_ON; sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL); + if (host-vmmc) + regulator_enable(host-vmmc); pr_info(DRIVER_NAME : Switching to 1.8V signalling voltage failed, retrying with S18R set to 0\n); -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mmc: only support voltage (vdd) that regulator agree's with
Hi Chris,
On Fri, Jun 29, 2012 at 07:19:19PM -0400, Chris Ball wrote:
Hi,
On Fri, Jun 08 2012, philipspatc...@gmail.com wrote:
From: Philip Rakity prak...@marvell.com
If we are using a regulator the SD Host Controller and the
regulator should agree about the voltages supported. Use
the common subset that is supported.
Signed-off-by: Philip Rakity prak...@marvell.com
This breaks the build when CONFIG_REGULATOR=n -- I've applied what looks
like the correct fix to me:
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index eae7c3c..caba999 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2832,51 +2832,53 @@ int sdhci_add_host(struct sdhci_host *host)
+#ifdef CONFIG_REGULATOR
/*
* According to SD Host Controller spec v3.00, if the Host System
* can afford more than 150mA, Host Driver should set XPC to 1. Also
* the value is meaningful only if Voltage Support in the Capabilities
* register is set. The actual current value is 4 times the register
* value.
*/
The above comments does not belong to regulator code, it should be
placed on top of the max_current_caps variable like this:
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index caba999..f76736b 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2845,13 +2845,6 @@ int sdhci_add_host(struct sdhci_host *host)
}
#ifdef CONFIG_REGULATOR
- /*
-* According to SD Host Controller spec v3.00, if the Host System
-* can afford more than 150mA, Host Driver should set XPC to 1. Also
-* the value is meaningful only if Voltage Support in the Capabilities
-* register is set. The actual current value is 4 times the register
-* value.
-*/
if (host-vmmc) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
@@ -2868,6 +2861,13 @@ int sdhci_add_host(struct sdhci_host *host)
}
#endif /* CONFIG_REGULATOR */
+ /*
+* According to SD Host Controller spec v3.00, if the Host System
+* can afford more than 150mA, Host Driver should set XPC to 1. Also
+* the value is meaningful only if Voltage Support in the Capabilities
+* register is set. The actual current value is 4 times the register
+* value.
+*/
max_current_caps = sdhci_readl(host, SDHCI_MAX_CURRENT);
if (!max_current_caps host-vmmc) {
u32 curr = regulator_get_current_limit(host-vmmc);
Thanks,
Aaron
if (host-vmmc) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
caps[0] = ~SDHCI_CAN_VDD_330;
ret = regulator_is_supported_voltage(host-vmmc, 300,
300);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_300)))
caps[0] = ~SDHCI_CAN_VDD_300;
ret = regulator_is_supported_voltage(host-vmmc, 180,
180);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_180)))
caps[0] = ~SDHCI_CAN_VDD_180;
}
+#endif /* CONFIG_REGULATOR */
--
Chris Ball c...@laptop.org 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] SD current limit setting fix
The following 2 patches fix setting of SD current limit Aaron Lu (2): mmc: core: Simplify and fix for SD switch processing mmc: sd: Fix sd current limit setting drivers/mmc/core/sd.c| 144 +++ drivers/mmc/host/sdhci.c | 28 +++-- include/linux/mmc/host.h | 16 -- 3 files changed, 105 insertions(+), 83 deletions(-) -- 1.7.11.1.3.g4c8a9db -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] mmc: core: Simplify and fix for SD switch processing
In mmc_read_switch, just do a one time mode 0 switch command to get the
support bits information, no need to do multiple times as the support
bits do not change with different arguments.
And no need to check current limit support bits, as these bits is
fixed according to the signal voltage. If the signal voltage is 1.8V,
the support bits would be 0xf and if the signal voltage is 3.3V, the
support bits would be 0x01. We will check host's ability to set the
current limit.
Signed-off-by: Aaron Lu aaron...@amd.com
---
drivers/mmc/core/sd.c | 94 ---
1 file changed, 22 insertions(+), 72 deletions(-)
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index b0b9e37..ae72d6e 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -290,8 +290,12 @@ static int mmc_read_switch(struct mmc_card *card)
return -ENOMEM;
}
- /* Find out the supported Bus Speed Modes. */
- err = mmc_sd_switch(card, 0, 0, 1, status);
+ /*
+* Find out the card's support bits with a mode 0 operation.
+* The argument does not matter, as the support bits do not
+* change with the arguments.
+*/
+ err = mmc_sd_switch(card, 0, 0, 0, status);
if (err) {
/*
* If the host or the card can't do the switch,
@@ -312,46 +316,8 @@ static int mmc_read_switch(struct mmc_card *card)
if (card-scr.sda_spec3) {
card-sw_caps.sd3_bus_mode = status[13];
-
- /* Find out Driver Strengths supported by the card */
- err = mmc_sd_switch(card, 0, 2, 1, status);
- if (err) {
- /*
-* If the host or the card can't do the switch,
-* fail more gracefully.
-*/
- if (err != -EINVAL err != -ENOSYS err != -EFAULT)
- goto out;
-
- pr_warning(%s: problem reading
- Driver Strength.\n,
- mmc_hostname(card-host));
- err = 0;
-
- goto out;
- }
-
+ /* Driver Strengths supported by the card */
card-sw_caps.sd3_drv_type = status[9];
-
- /* Find out Current Limits supported by the card */
- err = mmc_sd_switch(card, 0, 3, 1, status);
- if (err) {
- /*
-* If the host or the card can't do the switch,
-* fail more gracefully.
-*/
- if (err != -EINVAL err != -ENOSYS err != -EFAULT)
- goto out;
-
- pr_warning(%s: problem reading
- Current Limit.\n,
- mmc_hostname(card-host));
- err = 0;
-
- goto out;
- }
-
- card-sw_caps.sd3_curr_limit = status[7];
}
out:
@@ -560,41 +526,24 @@ static int sd_set_current_limit(struct mmc_card *card, u8
*status)
* Current limit switch is only defined for SDR50, SDR104, and DDR50
* bus speed modes. For other bus speed modes, we do not change the
* current limit.
+* We only check host's capability here, if we set a limit that is
+* higher than the card's maximum current, the card will be using its
+* maximum current, e.g. if the card's maximum current is 300ma, and
+* when we set current limit to 200ma, the card will draw 200ma, and
+* when we set current limit to 400/600/800ma, the card will draw its
+* maximum 300ma from the host.
*/
if ((card-sd_bus_speed == UHS_SDR50_BUS_SPEED) ||
(card-sd_bus_speed == UHS_SDR104_BUS_SPEED) ||
(card-sd_bus_speed == UHS_DDR50_BUS_SPEED)) {
- if (card-host-caps MMC_CAP_MAX_CURRENT_800) {
- if (card-sw_caps.sd3_curr_limit SD_MAX_CURRENT_800)
- current_limit = SD_SET_CURRENT_LIMIT_800;
- else if (card-sw_caps.sd3_curr_limit
- SD_MAX_CURRENT_600)
- current_limit = SD_SET_CURRENT_LIMIT_600;
- else if (card-sw_caps.sd3_curr_limit
- SD_MAX_CURRENT_400)
- current_limit = SD_SET_CURRENT_LIMIT_400;
- else if (card-sw_caps.sd3_curr_limit
- SD_MAX_CURRENT_200)
- current_limit = SD_SET_CURRENT_LIMIT_200;
- } else if (card-host-caps MMC_CAP_MAX_CURRENT_600) {
- if (card-sw_caps.sd3_curr_limit SD_MAX_CURRENT_600
[PATCH 2/2] mmc: sd: Fix sd current limit setting
Host has different current capabilities at different voltages, we need
to record these settings seperately. Before set current limit for the sd
card, find out the current voltage first and then find out the current
capabilities of the host to set the limit.
Signed-off-by: Aaron Lu aaron...@amd.com
---
drivers/mmc/core/sd.c| 58 ++--
drivers/mmc/host/sdhci.c | 28 +++
include/linux/mmc/host.h | 16 +
3 files changed, 87 insertions(+), 15 deletions(-)
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index ae72d6e..4b4cf4d 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -521,11 +521,39 @@ static int sd_set_current_limit(struct mmc_card *card, u8
*status)
{
int current_limit = SD_SET_CURRENT_NO_CHANGE;
int err;
+ u8 voltage;
/*
* Current limit switch is only defined for SDR50, SDR104, and DDR50
* bus speed modes. For other bus speed modes, we do not change the
* current limit.
+*/
+ if ((card-sd_bus_speed != UHS_SDR50_BUS_SPEED)
+ (card-sd_bus_speed != UHS_SDR104_BUS_SPEED)
+ (card-sd_bus_speed != UHS_DDR50_BUS_SPEED))
+ return 0;
+
+ /*
+* Host has different current capabilities when operating at
+* different voltages, so find out the current voltage first.
+*/
+ switch (1 card-host-ios.vdd) {
+ case MMC_VDD_165_195:
+ voltage = 0; /* host's voltage is 1.8V */
+ break;
+ case MMC_VDD_29_30:
+ case MMC_VDD_30_31:
+ voltage = 1; /* host's voltage is 3.0V */
+ break;
+ case MMC_VDD_32_33:
+ case MMC_VDD_33_34:
+ voltage = 2; /* host's voltage is 3.3V */
+ break;
+ default:
+ BUG(); /* host's voltage is invalid */
+ }
+
+ /*
* We only check host's capability here, if we set a limit that is
* higher than the card's maximum current, the card will be using its
* maximum current, e.g. if the card's maximum current is 300ma, and
@@ -533,16 +561,32 @@ static int sd_set_current_limit(struct mmc_card *card, u8
*status)
* when we set current limit to 400/600/800ma, the card will draw its
* maximum 300ma from the host.
*/
- if ((card-sd_bus_speed == UHS_SDR50_BUS_SPEED) ||
- (card-sd_bus_speed == UHS_SDR104_BUS_SPEED) ||
- (card-sd_bus_speed == UHS_DDR50_BUS_SPEED)) {
- if (card-host-caps MMC_CAP_MAX_CURRENT_800)
+ if (voltage == 0) {
+ if (card-host-caps MMC_CAP_MAX_CURRENT_800_180)
+ current_limit = SD_SET_CURRENT_LIMIT_800;
+ else if (card-host-caps MMC_CAP_MAX_CURRENT_600_180)
+ current_limit = SD_SET_CURRENT_LIMIT_600;
+ else if (card-host-caps MMC_CAP_MAX_CURRENT_400_180)
+ current_limit = SD_SET_CURRENT_LIMIT_400;
+ else if (card-host-caps MMC_CAP_MAX_CURRENT_200_180)
+ current_limit = SD_SET_CURRENT_LIMIT_200;
+ } else if (voltage == 1) {
+ if (card-host-caps MMC_CAP_MAX_CURRENT_800_300)
+ current_limit = SD_SET_CURRENT_LIMIT_800;
+ else if (card-host-caps MMC_CAP_MAX_CURRENT_600_300)
+ current_limit = SD_SET_CURRENT_LIMIT_600;
+ else if (card-host-caps MMC_CAP_MAX_CURRENT_400_300)
+ current_limit = SD_SET_CURRENT_LIMIT_400;
+ else if (card-host-caps MMC_CAP_MAX_CURRENT_200_300)
+ current_limit = SD_SET_CURRENT_LIMIT_200;
+ } else if (voltage == 2) {
+ if (card-host-caps MMC_CAP_MAX_CURRENT_800_330)
current_limit = SD_SET_CURRENT_LIMIT_800;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_600)
+ else if (card-host-caps MMC_CAP_MAX_CURRENT_600_330)
current_limit = SD_SET_CURRENT_LIMIT_600;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_400)
+ else if (card-host-caps MMC_CAP_MAX_CURRENT_400_330)
current_limit = SD_SET_CURRENT_LIMIT_400;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_200)
+ else if (card-host-caps MMC_CAP_MAX_CURRENT_200_330)
current_limit = SD_SET_CURRENT_LIMIT_200;
}
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index caba999..00c2cbb 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2897,6 +2897,16 @@ int sdhci_add_host(struct sdhci_host *host)
if (max_current_330 150)
mmc-caps |= MMC_CAP_SET_XPC_330
[PATCH v2] mmc: sdhci: fix incorrect command used in tuning
V2:
Fix for SDIO case: both SD and SDIO cards use cmd19 while eMMC use cmd21.
V1:
For SD hosts using retuning mode 1, when retuning timer expired, it will
need to do retuning in sdhci_request before processing the actual
request. But the retuning command is fixed: cmd19 for SD card and cmd21
for eMMC card, so we can't use the original request's command to do the
tuning.
And since the tuning command depends on the card type atteched to the
host, we will need to know the card type to use the correct tuning
command.
Cc: stable sta...@vger.kernel.org [3.3+]
Signed-off-by: Aaron Lu aaron...@amd.com
---
drivers/mmc/host/sdhci.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index f76736b..4e53e6b 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -27,6 +27,7 @@
#include linux/mmc/mmc.h
#include linux/mmc/host.h
+#include linux/mmc/card.h
#include sdhci.h
@@ -1245,6 +1246,7 @@ static void sdhci_request(struct mmc_host *mmc, struct
mmc_request *mrq)
struct sdhci_host *host;
bool present;
unsigned long flags;
+ u32 tuning_opcode;
host = mmc_priv(mmc);
@@ -1292,8 +1294,12 @@ static void sdhci_request(struct mmc_host *mmc, struct
mmc_request *mrq)
*/
if ((host-flags SDHCI_NEEDS_RETUNING)
!(present_state (SDHCI_DOING_WRITE | SDHCI_DOING_READ))) {
+ /* eMMC uses cmd21 while sd and sdio use cmd19 */
+ tuning_opcode = mmc-card-type == MMC_TYPE_MMC ?
+ MMC_SEND_TUNING_BLOCK_HS200 :
+ MMC_SEND_TUNING_BLOCK;
spin_unlock_irqrestore(host-lock, flags);
- sdhci_execute_tuning(mmc, mrq-cmd-opcode);
+ sdhci_execute_tuning(mmc, tuning_opcode);
spin_lock_irqsave(host-lock, flags);
/* Restore original mmc_request structure */
--
1.7.11.1.3.g4c8a9db
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mmc: sdhci: fix incorrect command used in tuning
Hi,
On Tue, Jul 03, 2012 at 03:28:28PM +0530, Girish K S wrote:
On 3 July 2012 14:57, Aaron Lu aaron...@amd.com wrote:
V2:
Fix for SDIO case: both SD and SDIO cards use cmd19 while eMMC use cmd21.
V1:
For SD hosts using retuning mode 1, when retuning timer expired, it will
need to do retuning in sdhci_request before processing the actual
request. But the retuning command is fixed: cmd19 for SD card and cmd21
for eMMC card, so we can't use the original request's command to do the
tuning.
And since the tuning command depends on the card type atteched to the
host, we will need to know the card type to use the correct tuning
command.
Cc: stable sta...@vger.kernel.org [3.3+]
Signed-off-by: Aaron Lu aaron...@amd.com
---
drivers/mmc/host/sdhci.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index f76736b..4e53e6b 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -27,6 +27,7 @@
#include linux/mmc/mmc.h
#include linux/mmc/host.h
+#include linux/mmc/card.h
#include sdhci.h
@@ -1245,6 +1246,7 @@ static void sdhci_request(struct mmc_host *mmc,
struct mmc_request *mrq)
struct sdhci_host *host;
bool present;
unsigned long flags;
+ u32 tuning_opcode;
host = mmc_priv(mmc);
@@ -1292,8 +1294,12 @@ static void sdhci_request(struct mmc_host *mmc,
struct mmc_request *mrq)
*/
if ((host-flags SDHCI_NEEDS_RETUNING)
!(present_state (SDHCI_DOING_WRITE |
SDHCI_DOING_READ))) {
+ /* eMMC uses cmd21 while sd and sdio use cmd19 */
+ tuning_opcode = mmc-card-type == MMC_TYPE_MMC ?
+ MMC_SEND_TUNING_BLOCK_HS200 :
+ MMC_SEND_TUNING_BLOCK;
spin_unlock_irqrestore(host-lock, flags);
- sdhci_execute_tuning(mmc, mrq-cmd-opcode);
+ sdhci_execute_tuning(mmc, tuning_opcode);
dont you think the previous implementation does the same. It is
already handled by introducing the 2nd parameter.
Suppose the following scenario:
mmc_start_request (e.g. mrq-cmd-opcode is 18 for this call)
- host-ops-request
(sdhci's retuning timer expired, the flag SDHCI_NEEDS_RETUNING is set)
- sdhci_request
- sdhci_execute_tuning will be called before processing the
actual request due to retuning's requirement, but with the wrong command
opcode(cmd18) instead of cmd19 for sd/sdio or cmd21 for emmc.
The problem is with retuning, for normal explicit calls of
sdhci_execute_tuning, there is no problem with the code. But when
retuning is required, sdhci_execute_retuning will be executed implicitly
to the above layer and we have to use the right tuning command instead of
the current processing command, which can be any of the valid sd/sdio/mmc
commands.
-Aaron
spin_lock_irqsave(host-lock, flags);
/* Restore original mmc_request structure */
--
1.7.11.1.3.g4c8a9db
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mmc: sdhci: fix incorrect command used in tuning
Hi,
On Tue, Jul 03, 2012 at 05:32:36PM +0530, Girish K S wrote:
@@ -1292,8 +1294,12 @@ static void sdhci_request(struct mmc_host *mmc,
struct mmc_request *mrq)
*/
if ((host-flags SDHCI_NEEDS_RETUNING)
!(present_state (SDHCI_DOING_WRITE |
SDHCI_DOING_READ))) {
+ /* eMMC uses cmd21 while sd and sdio use cmd19
*/
+ tuning_opcode = mmc-card-type == MMC_TYPE_MMC
?
+ MMC_SEND_TUNING_BLOCK_HS200 :
+ MMC_SEND_TUNING_BLOCK;
spin_unlock_irqrestore(host-lock, flags);
- sdhci_execute_tuning(mmc, mrq-cmd-opcode);
+ sdhci_execute_tuning(mmc, tuning_opcode);
dont you think the previous implementation does the same. It is
already handled by introducing the 2nd parameter.
Suppose the following scenario:
mmc_start_request (e.g. mrq-cmd-opcode is 18 for this call)
- host-ops-request
(sdhci's retuning timer expired, the flag SDHCI_NEEDS_RETUNING is set)
- sdhci_request
- sdhci_execute_tuning will be called before processing the
actual request due to retuning's requirement, but with the wrong command
opcode(cmd18) instead of cmd19 for sd/sdio or cmd21 for emmc.
The problem is with retuning, for normal explicit calls of
sdhci_execute_tuning, there is no problem with the code. But when
retuning is required, sdhci_execute_retuning will be executed implicitly
to the above layer and we have to use the right tuning command instead of
the current processing command, which can be any of the valid sd/sdio/mmc
commands.
Thanks for the explanation. is it possible to make a separate local
function for this. Since mmc_host_ops has a member .execute_tuning
which takes 2 parameters that are called from respective sd/mmc/sdio
card files. there might be conflict
Sorry i just read it wrongly (function has only 1 param).
So do you have any other problems with this patch?
If not, can I have your ack on this patch?
Thanks,
Aaron
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mmc: sdhci: fix incorrect command used in tuning
Hi Chris, On Tue, Jul 03, 2012 at 08:37:33PM -0400, Chris Ball wrote: Hi, On Tue, Jul 03 2012, Aaron Lu wrote: V2: Fix for SDIO case: both SD and SDIO cards use cmd19 while eMMC use cmd21. V1: For SD hosts using retuning mode 1, when retuning timer expired, it will need to do retuning in sdhci_request before processing the actual request. But the retuning command is fixed: cmd19 for SD card and cmd21 for eMMC card, so we can't use the original request's command to do the tuning. And since the tuning command depends on the card type atteched to the host, we will need to know the card type to use the correct tuning command. Cc: stable sta...@vger.kernel.org [3.3+] Signed-off-by: Aaron Lu aaron...@amd.com --- Thanks, pushed to mmc-next for 3.6 with Philip's Reviewed-by. For next time, please put the patch changelog at this point in the patch (after ---) so that it doesn't show up in the recorded commit message. Also, please don't e-mail the patch to stable@ before it has entered mainline. They'll pick it up from the CC in the commit message when Linus merges it, they don't want a copy of the patch before then. OK, thanks for pointing this out, will follow this in the future. -Aaron Thanks! - Chris. -- Chris Ball c...@laptop.org 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 majord...@vger.kernel.org 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/2] mmc: sdhci: A new flag SDHCI_USING_RETUNING_TIMER
Add a new flag of SDHCI_USING_RETUNING_TIMER to represent if the host
is using a retuning timer for the card inserted.
This flag is set when the host does tuning the first time for the card
and the host's retuning mode is 1. This flag is used afterwards whenever
needs to decide if the host is currently using a retuning timer.
This flag is cleared when the card is removed in sdhci_reinit.
The set/clear of the flag and the start/stop of the retuning timer is
associated with the card's init/remove time, so there is no need to
touch it when the host is to be removed as at that time the card should
have already been removed.
Signed-off-by: Aaron Lu aaron...@amd.com
---
v3:
Change the macro name from SDHCI_NEEDS_RETUNING_TIMER to
SDHCI_USING_RETUNING_TIMER for better description of the flag as suggested
by Chris Ball.
v2:
v1 mistakenly removed the line del_timer_sync(host-timer) in
sdhci_remove_host, fix this by adding back the code.
drivers/mmc/host/sdhci.c | 30 --
include/linux/mmc/sdhci.h | 1 +
2 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index b0a5629..3ec4182 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -250,8 +250,9 @@ static void sdhci_reinit(struct sdhci_host *host)
* applicable to UHS-I cards. So reset these fields to their initial
* value when card is removed.
*/
- if (host-version = SDHCI_SPEC_300 host-tuning_count
- host-tuning_mode == SDHCI_TUNING_MODE_1) {
+ if (host-flags SDHCI_USING_RETUNING_TIMER) {
+ host-flags = ~SDHCI_USING_RETUNING_TIMER;
+
del_timer_sync(host-tuning_timer);
host-flags = ~SDHCI_NEEDS_RETUNING;
host-mmc-max_blk_count =
@@ -1873,6 +1874,7 @@ out:
*/
if (!(host-flags SDHCI_NEEDS_RETUNING) host-tuning_count
(host-tuning_mode == SDHCI_TUNING_MODE_1)) {
+ host-flags |= SDHCI_USING_RETUNING_TIMER;
mod_timer(host-tuning_timer, jiffies +
host-tuning_count * HZ);
/* Tuning mode 1 limits the maximum data length to 4MB */
@@ -1890,10 +1892,10 @@ out:
* try tuning again at a later time, when the re-tuning timer expires.
* So for these controllers, we return 0. Since there might be other
* controllers who do not have this capability, we return error for
-* them.
+* them. SDHCI_USING_RETUNING_TIMER means the host is currently using
+* a retuning timer to do the retuning for the card.
*/
- if (err host-tuning_count
- host-tuning_mode == SDHCI_TUNING_MODE_1)
+ if (err (host-flags SDHCI_USING_RETUNING_TIMER))
err = 0;
sdhci_clear_set_irqs(host, SDHCI_INT_DATA_AVAIL, ier);
@@ -2400,7 +2402,6 @@ out:
int sdhci_suspend_host(struct sdhci_host *host)
{
int ret;
- bool has_tuning_timer;
if (host-ops-platform_suspend)
host-ops-platform_suspend(host);
@@ -2408,16 +2409,14 @@ int sdhci_suspend_host(struct sdhci_host *host)
sdhci_disable_card_detection(host);
/* Disable tuning since we are suspending */
- has_tuning_timer = host-version = SDHCI_SPEC_300
- host-tuning_count host-tuning_mode == SDHCI_TUNING_MODE_1;
- if (has_tuning_timer) {
+ if (host-flags SDHCI_USING_RETUNING_TIMER) {
del_timer_sync(host-tuning_timer);
host-flags = ~SDHCI_NEEDS_RETUNING;
}
ret = mmc_suspend_host(host-mmc);
if (ret) {
- if (has_tuning_timer) {
+ if (host-flags SDHCI_USING_RETUNING_TIMER) {
host-flags |= SDHCI_NEEDS_RETUNING;
mod_timer(host-tuning_timer, jiffies +
host-tuning_count * HZ);
@@ -2468,8 +2467,7 @@ int sdhci_resume_host(struct sdhci_host *host)
host-ops-platform_resume(host);
/* Set the re-tuning expiration flag */
- if ((host-version = SDHCI_SPEC_300) host-tuning_count
- (host-tuning_mode == SDHCI_TUNING_MODE_1))
+ if (host-flags SDHCI_USING_RETUNING_TIMER)
host-flags |= SDHCI_NEEDS_RETUNING;
return ret;
@@ -2508,8 +2506,7 @@ int sdhci_runtime_suspend_host(struct sdhci_host *host)
int ret = 0;
/* Disable tuning since we are suspending */
- if (host-version = SDHCI_SPEC_300
- host-tuning_mode == SDHCI_TUNING_MODE_1) {
+ if (host-flags SDHCI_USING_RETUNING_TIMER) {
del_timer_sync(host-tuning_timer);
host-flags = ~SDHCI_NEEDS_RETUNING;
}
@@ -2550,8 +2547,7 @@ int sdhci_runtime_resume_host(struct sdhci_host *host)
sdhci_do_enable_preset_value(host, true);
/* Set the re-tuning expiration flag
[PATCH v2 2/2] mmc: sd: Fix sd current limit setting
Host has different current capabilities at different voltages, we need
to record these settings seperately. The defined voltages are 1.8/3.0/3.3.
For other voltages, we do not touch current limit setting.
Before set current limit for the sd card, find out the host's operating
voltage first and then find out the current capabilities of the host at
that voltage to set the current limit.
Signed-off-by: Aaron Lu aaron...@amd.com
---
v2:
Do not call BUG() when the host's voltage is not supported as suggested by
Chris Ball.
Do not use 0/1/2 to represent the host's voltage as suggested by Philip Rakity.
drivers/mmc/core/sd.c| 44 +---
drivers/mmc/host/sdhci.c | 28
include/linux/mmc/host.h | 16
3 files changed, 73 insertions(+), 15 deletions(-)
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 8460568..312b78d 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -521,11 +521,25 @@ static int sd_set_current_limit(struct mmc_card *card, u8
*status)
{
int current_limit = SD_SET_CURRENT_NO_CHANGE;
int err;
+ u32 voltage;
/*
* Current limit switch is only defined for SDR50, SDR104, and DDR50
* bus speed modes. For other bus speed modes, we do not change the
* current limit.
+*/
+ if ((card-sd_bus_speed != UHS_SDR50_BUS_SPEED)
+ (card-sd_bus_speed != UHS_SDR104_BUS_SPEED)
+ (card-sd_bus_speed != UHS_DDR50_BUS_SPEED))
+ return 0;
+
+ /*
+* Host has different current capabilities when operating at
+* different voltages, so find out the current voltage first.
+*/
+ voltage = 1 card-host-ios.vdd;
+
+ /*
* We only check host's capability here, if we set a limit that is
* higher than the card's maximum current, the card will be using its
* maximum current, e.g. if the card's maximum current is 300ma, and
@@ -533,16 +547,32 @@ static int sd_set_current_limit(struct mmc_card *card, u8
*status)
* when we set current limit to 400/600/800ma, the card will draw its
* maximum 300ma from the host.
*/
- if ((card-sd_bus_speed == UHS_SDR50_BUS_SPEED) ||
- (card-sd_bus_speed == UHS_SDR104_BUS_SPEED) ||
- (card-sd_bus_speed == UHS_DDR50_BUS_SPEED)) {
- if (card-host-caps MMC_CAP_MAX_CURRENT_800)
+ if (voltage == MMC_VDD_165_195) {
+ if (card-host-caps MMC_CAP_MAX_CURRENT_800_180)
+ current_limit = SD_SET_CURRENT_LIMIT_800;
+ else if (card-host-caps MMC_CAP_MAX_CURRENT_600_180)
+ current_limit = SD_SET_CURRENT_LIMIT_600;
+ else if (card-host-caps MMC_CAP_MAX_CURRENT_400_180)
+ current_limit = SD_SET_CURRENT_LIMIT_400;
+ else if (card-host-caps MMC_CAP_MAX_CURRENT_200_180)
+ current_limit = SD_SET_CURRENT_LIMIT_200;
+ } else if (voltage (MMC_VDD_29_30 | MMC_VDD_30_31)) {
+ if (card-host-caps MMC_CAP_MAX_CURRENT_800_300)
+ current_limit = SD_SET_CURRENT_LIMIT_800;
+ else if (card-host-caps MMC_CAP_MAX_CURRENT_600_300)
+ current_limit = SD_SET_CURRENT_LIMIT_600;
+ else if (card-host-caps MMC_CAP_MAX_CURRENT_400_300)
+ current_limit = SD_SET_CURRENT_LIMIT_400;
+ else if (card-host-caps MMC_CAP_MAX_CURRENT_200_300)
+ current_limit = SD_SET_CURRENT_LIMIT_200;
+ } else if (voltage (MMC_VDD_32_33 | MMC_VDD_33_34)) {
+ if (card-host-caps MMC_CAP_MAX_CURRENT_800_330)
current_limit = SD_SET_CURRENT_LIMIT_800;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_600)
+ else if (card-host-caps MMC_CAP_MAX_CURRENT_600_330)
current_limit = SD_SET_CURRENT_LIMIT_600;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_400)
+ else if (card-host-caps MMC_CAP_MAX_CURRENT_400_330)
current_limit = SD_SET_CURRENT_LIMIT_400;
- else if (card-host-caps MMC_CAP_MAX_CURRENT_200)
+ else if (card-host-caps MMC_CAP_MAX_CURRENT_200_330)
current_limit = SD_SET_CURRENT_LIMIT_200;
}
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 3ec4182..d89e97c 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2913,6 +2913,16 @@ int sdhci_add_host(struct sdhci_host *host)
if (max_current_330 150)
mmc-caps |= MMC_CAP_SET_XPC_330;
+
+ /* Maximum current capabilities of the host at 3.3V */
+ if (max_current_330 = 800)
+ mmc-caps |= MMC_CAP_MAX_CURRENT_800_330;
+ else
[PATCH 0/2] Fixes for tuning stuffs
The following 2 patches fixed a bug related the tuning stuffs and refactored the code to make it simpler and easier to read and understand. Aaron Lu (2): mmc: sdhci: restore host settings when card is removed mmc: sdhci: A new flag SDHCI_NEEDS_RETUNING_TIMER drivers/mmc/host/sdhci.c | 39 +-- include/linux/mmc/sdhci.h | 1 + 2 files changed, 22 insertions(+), 18 deletions(-) -- 1.7.11.1.3.g4c8a9db -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] mmc: sdhci: restore host settings when card is removed
Some of the host settings are affected by different cards inserted, e.g.
when an UHS-I card is inserted, the SDHCI_NEEDS_RETUING flag might be
set when the tuning timer expired and host's max_blk_count will be
reduced to make sure the data transfer for a command does not exceed 4MiB
to meet the retuning mode 1's requirement.
When the card is removed, we should restore the original setting of the
host since we can't be sure the next card being inserted will still be
an UHS-I card that needs tuning. The original setting include its
max_blk_count and no set of the flag of SDHCI_NEEDS_RETUNING.
Signed-off-by: Aaron Lu aaron...@amd.com
---
drivers/mmc/host/sdhci.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index ff522ec..7e182ad 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -245,6 +245,18 @@ static void sdhci_init(struct sdhci_host *host, int soft)
static void sdhci_reinit(struct sdhci_host *host)
{
sdhci_init(host, 0);
+ /*
+* Retuning stuffs are affected by different cards inserted and only
+* applicable to UHS-I cards. So reset these fields to their initial
+* value when card is removed.
+*/
+ if (host-version = SDHCI_SPEC_300 host-tuning_count
+ host-tuning_mode == SDHCI_TUNING_MODE_1) {
+ del_timer_sync(host-tuning_timer);
+ host-flags = ~SDHCI_NEEDS_RETUNING;
+ host-mmc-max_blk_count =
+ (host-quirks SDHCI_QUIRK_NO_MULTIBLOCK) ? 1 : 65535;
+ }
sdhci_enable_card_detection(host);
}
--
1.7.11.1.3.g4c8a9db
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] mmc: sdhci: A new flag SDHCI_NEEDS_RETUNING_TIMER
Add a new flag of SDHCI_NEEDS_RETUNING_TIMER to represent if the host
needs retuning timer currently when driving the card inserted.
This flag is set when the host does tuning the first time for the card
and is used afterwards whenever needs to decide if the host is currently
using a retuning timer.
This flag is cleared when the card is removed in sdhci_reinit.
The set/clear of the flag and the start/stop of the retuning timer is
associated with the card's init/remove time, so there is no need to
touch it when the host is to be removed as at that time the card should
have already been removed.
Signed-off-by: Aaron Lu aaron...@amd.com
---
drivers/mmc/host/sdhci.c | 31 +++
include/linux/mmc/sdhci.h | 1 +
2 files changed, 12 insertions(+), 20 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 7e182ad..9918bcc 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -250,8 +250,9 @@ static void sdhci_reinit(struct sdhci_host *host)
* applicable to UHS-I cards. So reset these fields to their initial
* value when card is removed.
*/
- if (host-version = SDHCI_SPEC_300 host-tuning_count
- host-tuning_mode == SDHCI_TUNING_MODE_1) {
+ if (host-flags SDHCI_NEEDS_RETUNING_TIMER) {
+ host-flags = ~SDHCI_NEEDS_RETUNING_TIMER;
+
del_timer_sync(host-tuning_timer);
host-flags = ~SDHCI_NEEDS_RETUNING;
host-mmc-max_blk_count =
@@ -1872,6 +1873,7 @@ out:
*/
if (!(host-flags SDHCI_NEEDS_RETUNING) host-tuning_count
(host-tuning_mode == SDHCI_TUNING_MODE_1)) {
+ host-flags |= SDHCI_NEEDS_RETUNING_TIMER;
mod_timer(host-tuning_timer, jiffies +
host-tuning_count * HZ);
/* Tuning mode 1 limits the maximum data length to 4MB */
@@ -1889,10 +1891,9 @@ out:
* try tuning again at a later time, when the re-tuning timer expires.
* So for these controllers, we return 0. Since there might be other
* controllers who do not have this capability, we return error for
-* them.
+* them. SDHCI_NEEDS_RETUNING_TIMER means the host supports re-tuning.
*/
- if (err host-tuning_count
- host-tuning_mode == SDHCI_TUNING_MODE_1)
+ if (err (host-flags SDHCI_NEEDS_RETUNING_TIMER))
err = 0;
sdhci_clear_set_irqs(host, SDHCI_INT_DATA_AVAIL, ier);
@@ -2399,7 +2400,6 @@ out:
int sdhci_suspend_host(struct sdhci_host *host)
{
int ret;
- bool has_tuning_timer;
if (host-ops-platform_suspend)
host-ops-platform_suspend(host);
@@ -2407,16 +2407,14 @@ int sdhci_suspend_host(struct sdhci_host *host)
sdhci_disable_card_detection(host);
/* Disable tuning since we are suspending */
- has_tuning_timer = host-version = SDHCI_SPEC_300
- host-tuning_count host-tuning_mode == SDHCI_TUNING_MODE_1;
- if (has_tuning_timer) {
+ if (host-flags SDHCI_NEEDS_RETUNING_TIMER) {
del_timer_sync(host-tuning_timer);
host-flags = ~SDHCI_NEEDS_RETUNING;
}
ret = mmc_suspend_host(host-mmc);
if (ret) {
- if (has_tuning_timer) {
+ if (host-flags SDHCI_NEEDS_RETUNING_TIMER) {
host-flags |= SDHCI_NEEDS_RETUNING;
mod_timer(host-tuning_timer, jiffies +
host-tuning_count * HZ);
@@ -2467,8 +2465,7 @@ int sdhci_resume_host(struct sdhci_host *host)
host-ops-platform_resume(host);
/* Set the re-tuning expiration flag */
- if ((host-version = SDHCI_SPEC_300) host-tuning_count
- (host-tuning_mode == SDHCI_TUNING_MODE_1))
+ if (host-flags SDHCI_NEEDS_RETUNING_TIMER)
host-flags |= SDHCI_NEEDS_RETUNING;
return ret;
@@ -2507,8 +2504,7 @@ int sdhci_runtime_suspend_host(struct sdhci_host *host)
int ret = 0;
/* Disable tuning since we are suspending */
- if (host-version = SDHCI_SPEC_300
- host-tuning_mode == SDHCI_TUNING_MODE_1) {
+ if (host-flags SDHCI_NEEDS_RETUNING_TIMER) {
del_timer_sync(host-tuning_timer);
host-flags = ~SDHCI_NEEDS_RETUNING;
}
@@ -2549,8 +2545,7 @@ int sdhci_runtime_resume_host(struct sdhci_host *host)
sdhci_do_enable_preset_value(host, true);
/* Set the re-tuning expiration flag */
- if ((host-version = SDHCI_SPEC_300) host-tuning_count
- (host-tuning_mode == SDHCI_TUNING_MODE_1))
+ if (host-flags SDHCI_NEEDS_RETUNING_TIMER)
host-flags |= SDHCI_NEEDS_RETUNING;
spin_lock_irqsave(host-lock, flags);
@@ -3097,10 +3092,6 @@ void sdhci_remove_host(struct sdhci_host *host, int dead
Re: [PATCH 2/2] mmc: sdhci: A new flag SDHCI_NEEDS_RETUNING_TIMER
Hi, On Fri, Jun 29, 2012 at 02:11:29AM -0400, Chris Ball wrote: Hi, On Fri, Jun 29 2012, Aaron Lu wrote: Add a new flag of SDHCI_NEEDS_RETUNING_TIMER to represent if the host needs retuning timer currently when driving the card inserted. This flag is set when the host does tuning the first time for the card and is used afterwards whenever needs to decide if the host is currently using a retuning timer. This flag is cleared when the card is removed in sdhci_reinit. The set/clear of the flag and the start/stop of the retuning timer is associated with the card's init/remove time, so there is no need to touch it when the host is to be removed as at that time the card should have already been removed. [...] @@ -3097,10 +3092,6 @@ void sdhci_remove_host(struct sdhci_host *host, int dead) free_irq(host-irq, host); - del_timer_sync(host-timer); - if (host-version = SDHCI_SPEC_300) - del_timer_sync(host-tuning_timer); - tasklet_kill(host-card_tasklet); tasklet_kill(host-finish_tasklet); The last paragraph of the commit message explains why you remove the del_timer_sync() call on the tuning_timer; but why do you remove it from the main (timeouts) host-timer too? Oops... My mistake, sorry for that. Will send v2. Thanks, Aaron Thanks, - Chris. -- Chris Ball c...@laptop.org 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 majord...@vger.kernel.org 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/2] Fixes for tuning stuffs
v2: v1 mistakenly removed the line del_timer_sync(host-timer) in sdhci_remove_host, fix this by adding back the code. v1: The following 2 patches fixed a bug related the tuning stuffs and refactored the code to make it simpler and easier to read and understand. -- 1.7.11.1.3.g4c8a9db -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mmc: sdhci: fix incorrect command used in tuning
For SD hosts using retuning mode 1, when retuning timer expired, it will
need to do retuning in sdhci_request before processing the actual
request. But the retuning command is fixed: cmd19 for SD card and cmd21
for eMMC card, so we can't use the original request's command to do the
tuning.
And since the tuning command depends on the card type atteched to the
host, we will need to know the card type to use the correct tuning
command.
Signed-off-by: Aaron Lu aaron...@amd.com
Cc: stable sta...@vger.kernel.org [3.3+]
---
drivers/mmc/host/sdhci.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index f4b8b4d..ff522ec 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -27,6 +27,7 @@
#include linux/mmc/mmc.h
#include linux/mmc/host.h
+#include linux/mmc/card.h
#include sdhci.h
@@ -1245,6 +1246,7 @@ static void sdhci_request(struct mmc_host *mmc, struct
mmc_request *mrq)
struct sdhci_host *host;
bool present;
unsigned long flags;
+ u32 tuning_opcode;
host = mmc_priv(mmc);
@@ -1292,8 +1294,11 @@ static void sdhci_request(struct mmc_host *mmc, struct
mmc_request *mrq)
*/
if ((host-flags SDHCI_NEEDS_RETUNING)
!(present_state (SDHCI_DOING_WRITE | SDHCI_DOING_READ))) {
+ tuning_opcode = mmc-card-type == MMC_TYPE_SD ?
+ MMC_SEND_TUNING_BLOCK :
+ MMC_SEND_TUNING_BLOCK_HS200;
spin_unlock_irqrestore(host-lock, flags);
- sdhci_execute_tuning(mmc, mrq-cmd-opcode);
+ sdhci_execute_tuning(mmc, tuning_opcode);
spin_lock_irqsave(host-lock, flags);
/* Restore original mmc_request structure */
--
1.7.11.2.gd284367
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] mmc: bus: print bus speed mode of UHS-I card
Hi,
On Fri, Mar 30, 2012 at 12:10:18PM +0530, Subhash Jadavani wrote:
When UHS-I card is detected also print the bus speed mode
in which UHS-I card will be running.
Signed-off-by: Subhash Jadavani subha...@codeaurora.org
Acked-by: Aaron Lu aaron...@amd.com
-Aaron
---
drivers/mmc/core/bus.c | 17 +++--
1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 5d011a3..3f60606 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -267,6 +267,15 @@ int mmc_add_card(struct mmc_card *card)
{
int ret;
const char *type;
+ const char *uhs_bus_speed_mode = ;
+ static const char *const uhs_speeds[] = {
+ [UHS_SDR12_BUS_SPEED] = SDR12 ,
+ [UHS_SDR25_BUS_SPEED] = SDR25 ,
+ [UHS_SDR50_BUS_SPEED] = SDR50 ,
+ [UHS_SDR104_BUS_SPEED] = SDR104 ,
+ [UHS_DDR50_BUS_SPEED] = DDR50 ,
+ };
+
dev_set_name(card-dev, %s:%04x, mmc_hostname(card-host),
card-rca);
@@ -296,6 +305,10 @@ int mmc_add_card(struct mmc_card *card)
break;
}
+ if (mmc_sd_card_uhs(card)
+ (card-sd_bus_speed ARRAY_SIZE(uhs_speeds)))
+ uhs_bus_speed_mode = uhs_speeds[card-sd_bus_speed];
+
if (mmc_host_is_spi(card-host)) {
pr_info(%s: new %s%s%s card on SPI\n,
mmc_hostname(card-host),
@@ -303,13 +316,13 @@ int mmc_add_card(struct mmc_card *card)
mmc_card_ddr_mode(card) ? DDR : ,
type);
} else {
- pr_info(%s: new %s%s%s%s card at address %04x\n,
+ pr_info(%s: new %s%s%s%s%s card at address %04x\n,
mmc_hostname(card-host),
mmc_card_uhs(card) ? ultra high speed :
(mmc_card_highspeed(card) ? high speed : ),
(mmc_card_hs200(card) ? HS200 : ),
mmc_card_ddr_mode(card) ? DDR : ,
- type, card-rca);
+ uhs_bus_speed_mode, type, card-rca);
}
#ifdef CONFIG_DEBUG_FS
--
1.7.1.1
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 1/1] mmc: bus: print bus speed mode of UHS-I card
Hi, On Mon, Mar 26, 2012 at 12:00:27PM +0530, Subhash Jadavani wrote: Actually we are already printing the HS200/DDR (for eMMC) bus speed mode along with card detected print. So why not bus speed mode for UHS-I cards. It really gives good impression to user what this card is capable of doing by just looking at this print. Correct we can this information from /sys/kernel/debug/mmc/ios but this needs extra effort than just looking at the kernel logs. But still please check if this point makes sense? Yes, I think this makes sense. -Aaron -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 1/1] mmc: bus: print bus speed mode of UHS-I card
Hi Subhash,
On Fri, Mar 23, 2012 at 11:26:36AM +0530, Subhash Jadavani wrote:
When UHS-I card is detected also print the bus speed mode
in which UHS-I card will be running.
The patch looks correct to me, except that I' m not sure if this is
needed. /sys/kernel/debug/mmc/ios also has such info.
-Aaron
Signed-off-by: Subhash Jadavani subha...@codeaurora.org
---
drivers/mmc/core/bus.c | 26 +-
1 files changed, 25 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 5d011a3..0517a91 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -267,6 +267,7 @@ int mmc_add_card(struct mmc_card *card)
{
int ret;
const char *type;
+ const char *uhs_bus_speed_mode = ;
dev_set_name(card-dev, %s:%04x, mmc_hostname(card-host),
card-rca);
@@ -296,6 +297,28 @@ int mmc_add_card(struct mmc_card *card)
break;
}
+ if (mmc_sd_card_uhs(card)) {
+ switch (card-sd_bus_speed) {
+ case UHS_SDR104_BUS_SPEED:
+ uhs_bus_speed_mode = SDR104 ;
+ break;
+ case UHS_SDR50_BUS_SPEED:
+ uhs_bus_speed_mode = SDR50 ;
+ break;
+ case UHS_DDR50_BUS_SPEED:
+ uhs_bus_speed_mode = DDR50 ;
+ break;
+ case UHS_SDR25_BUS_SPEED:
+ uhs_bus_speed_mode = SDR25 ;
+ break;
+ case UHS_SDR12_BUS_SPEED:
+ uhs_bus_speed_mode = SDR12 ;
+ break;
+ default:
+ uhs_bus_speed_mode = ;
+ break;
+ }
+ }
if (mmc_host_is_spi(card-host)) {
pr_info(%s: new %s%s%s card on SPI\n,
mmc_hostname(card-host),
@@ -303,12 +326,13 @@ int mmc_add_card(struct mmc_card *card)
mmc_card_ddr_mode(card) ? DDR : ,
type);
} else {
- pr_info(%s: new %s%s%s%s card at address %04x\n,
+ pr_info(%s: new %s%s%s%s%s card at address %04x\n,
mmc_hostname(card-host),
mmc_card_uhs(card) ? ultra high speed :
(mmc_card_highspeed(card) ? high speed : ),
(mmc_card_hs200(card) ? HS200 : ),
mmc_card_ddr_mode(card) ? DDR : ,
+ uhs_bus_speed_mode,
type, card-rca);
}
--
1.7.1.1
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mmc: sdhci: always reset all during resume
On Mon, Jan 30, 2012 at 02:27:19PM +0200, Adrian Hunter wrote:
During suspend the host controller may or may not be powered off.
In order to get the same result either way, always perform a
software reset all when resuming.
Signed-off-by: Adrian Hunter adrian.hun...@intel.com
Cc: Philip Rakity prak...@marvell.com
Cc: Aaron Lu aaron...@amd.com
Acked-by: Aaron Lu aaron...@amd.com
---
drivers/mmc/host/sdhci.c | 29 +++--
1 files changed, 11 insertions(+), 18 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 8d66706..ef2434c 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -219,31 +219,20 @@ static void sdhci_reset(struct sdhci_host *host, u8
mask)
}
}
-static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios);
-
-static void sdhci_init(struct sdhci_host *host, int soft)
+static void sdhci_init(struct sdhci_host *host)
{
- if (soft)
- sdhci_reset(host, SDHCI_RESET_CMD|SDHCI_RESET_DATA);
- else
- sdhci_reset(host, SDHCI_RESET_ALL);
+ sdhci_reset(host, SDHCI_RESET_ALL);
sdhci_clear_set_irqs(host, SDHCI_INT_ALL_MASK,
SDHCI_INT_BUS_POWER | SDHCI_INT_DATA_END_BIT |
SDHCI_INT_DATA_CRC | SDHCI_INT_DATA_TIMEOUT | SDHCI_INT_INDEX |
SDHCI_INT_END_BIT | SDHCI_INT_CRC | SDHCI_INT_TIMEOUT |
SDHCI_INT_DATA_END | SDHCI_INT_RESPONSE);
-
- if (soft) {
- /* force clock reconfiguration */
- host-clock = 0;
- sdhci_set_ios(host-mmc, host-mmc-ios);
- }
}
static void sdhci_reinit(struct sdhci_host *host)
{
- sdhci_init(host, 0);
+ sdhci_init(host);
sdhci_enable_card_detection(host);
}
@@ -2423,8 +2412,12 @@ int sdhci_resume_host(struct sdhci_host *host)
if (ret)
return ret;
- sdhci_init(host, (host-mmc-pm_flags MMC_PM_KEEP_POWER));
- mmiowb();
+ sdhci_init(host);
+
+ /* Force clock and power re-program */
+ host-pwr = 0;
+ host-clock = 0;
+ sdhci_do_set_ios(host, host-mmc-ios);
ret = mmc_resume_host(host-mmc);
sdhci_enable_card_detection(host);
@@ -2500,7 +2493,7 @@ int sdhci_runtime_resume_host(struct sdhci_host *host)
host-ops-enable_dma(host);
}
- sdhci_init(host, 0);
+ sdhci_init(host);
/* Force clock and power re-program */
host-pwr = 0;
@@ -2980,7 +2973,7 @@ int sdhci_add_host(struct sdhci_host *host)
host-vmmc = NULL;
}
- sdhci_init(host, 0);
+ sdhci_init(host);
#ifdef CONFIG_MMC_DEBUG
sdhci_dumpregs(host);
--
1.7.6.4
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4 v3] MMC/core: Add f_min to mmc_power_on()
Hi, On Fri, Jan 13, 2012 at 02:24:46AM +, Huang Changming-R66093 wrote: Hi, Chris, Could you have any comment about this patch? Can it go into 3.3 or 3.4? -Original Message- From: Huang Changming-R66093 Sent: Friday, December 09, 2011 10:54 AM To: linux-mmc@vger.kernel.org Cc: Huang Changming-R66093; Chris Ball Subject: [PATCH 1/4 v3] MMC/core: Add f_min to mmc_power_on() From: Jerry Huang chang-ming.hu...@freescale.com When f_init is zero, the SDHC can't work correctly. So f_min will replace f_init, when f_init is zero. What about setting f_init before your call to mmc_power_up? Signed-off-by: Jerry Huang chang-ming.hu...@freescale.com CC: Chris Ball c...@laptop.org --- changes for v2: - add the CC changes for v3: - enalbe the controller clock in platform, instead of core drivers/mmc/core/core.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index a08e6b1..2d40c04 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1253,7 +1253,10 @@ static void mmc_power_up(struct mmc_host *host) */ mmc_delay(10); - host-ios.clock = host-f_init; + if (host-f_init) + host-ios.clock = host-f_init; + else + host-ios.clock = host-f_min; host-ios.power_mode = MMC_POWER_ON; mmc_set_ios(host); -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
Hi,
On Fri, Jan 13, 2012 at 02:25:11AM +, Huang Changming-R66093 wrote:
Hi, Chris,
Could you have any comment about this patch?
Can it go into 3.3 or 3.4?
Thanks
Jerry Huang
-Original Message-
From: Huang Changming-R66093
Sent: Tuesday, December 27, 2011 4:01 PM
To: linux-mmc@vger.kernel.org
Cc: Huang Changming-R66093; Chris Ball
Subject: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
From: Jerry Huang chang-ming.hu...@freescale.com
In order to check whether the card has been removed, the function
mmc_send_status() will send command CMD13 to card and ask the card
to send its status register to sdhc driver, which will generate
many interrupts repeatedly and make the system performance bad.
For sd hosts, this should only happen for hosts which have
SDHCI_QUIRK_BROKEN_CARD_DETECTION set.
Therefore, add callback function get_cd() to check whether
the card has been removed when the driver has this callback function.
I don't quite get the meaning of cd, what does get_cd suppose to mean?
If the card is present, 1 will return, if the card is absent, 0 will
return.
If the controller will not support this feature, -ENOSYS will return.
What about get_present, return 0 for present, and return error code
otherwise like the alive function does.
Signed-off-by: Jerry Huang chang-ming.hu...@freescale.com
CC: Chris Ball c...@laptop.org
---
changes for v2:
- when controller don't support get_cd, return -ENOSYS
- add the CC
changes for v3:
- enalbe the controller clock in platform, instead of core
changes for v4:
- move the detect code to core.c according to the new structure
drivers/mmc/core/core.c | 10 --
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 6db6621..d570c72 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2060,7 +2060,7 @@ static int mmc_rescan_try_freq(struct mmc_host
*host, unsigned freq)
snip
int _mmc_detect_card_removed(struct mmc_host *host)
{
- int ret;
+ int ret = -ENOSYS;
if ((host-caps MMC_CAP_NONREMOVABLE) || !host-bus_ops-alive)
return 0;
@@ -2068,7 +2068,13 @@ int _mmc_detect_card_removed(struct mmc_host *host)
if (!host-card || mmc_card_removed(host-card))
return 1;
- ret = host-bus_ops-alive(host);
+ if (host-ops-get_cd) {
+ ret = host-ops-get_cd(host);
+ if (ret = 0)
+ ret = !ret;
+ }
+ if (ret 0)
+ ret = host-bus_ops-alive(host);
if (ret) {
mmc_card_set_removed(host-card);
pr_debug(%s: card remove detected\n, mmc_hostname(host));
--
1.7.5.4
And the code can be changed to something like:
if (host-ops-get_present)
ret = host-ops-get_present(host);
else
ret = host-bus_ops-alive(host);
if (ret) {
mmc_card_set_removed(host-card);
pr_debug(%s: card remove detected\n, mmc_hostname(host));
}
Does this make sense?
-Aaron
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4 v5] SDHCI: add sdhci_get_cd callback to detect the card
Hi,
On Fri, Jan 13, 2012 at 02:25:18AM +, Huang Changming-R66093 wrote:
Hi, Chris,
Could you have any comment about this patch?
Can it go into 3.3 or 3.4?
Thanks
Jerry Huang
-Original Message-
From: Huang Changming-R66093
Sent: Wednesday, December 14, 2011 10:18 AM
To: linux-mmc@vger.kernel.org
Cc: Huang Changming-R66093; Chris Ball
Subject: [PATCH 3/4 v5] SDHCI: add sdhci_get_cd callback to detect the
card
From: Jerry Huang chang-ming.hu...@freescale.com
Add callback function sdhci_get_cd to detect the card.
And one new callback added to implement the card detect in sdhci
struncture.
If special platform has the card detect callback, it will return the card
state, the value zero is for absent cardi and one is for present card.
If the controller don't support card detect, sdhci_get_cd will return -
ENOSYS.
Signed-off-by: Jerry Huang chang-ming.hu...@freescale.com
CC: Chris Ball c...@laptop.org
---
changes for v2:
- when controller don't support get_cd, return -ENOSYS
- add new callback for sdhci to detect the card
- add the CC
changes for v3:
- use pin_lock only when get_cd defined
changes for v4:
- enalbe the controller clock in platform, instead of core
changes for v5:
- remove the copyright
drivers/mmc/host/sdhci.c | 21 ++
drivers/mmc/host/sdhci.h |2 ++
2 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6d8eea3..fbe2f46 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1518,6 +1519,26 @@ static int sdhci_get_ro(struct mmc_host *mmc)
return ret;
}
snip
+/* Return values for the sdjco_get_cd callback:
+ * 0 for a absent card
+ * 1 for a present card
+ * -ENOSYS when not supported (equal to NULL callback)
+ */
+static int sdhci_get_cd(struct mmc_host *mmc)
+{
+ struct sdhci_host *host = mmc_priv(mmc);
+ unsigned long flags;
+ int present = -ENOSYS;
+
+ if (host-ops-get_cd) {
+ spin_lock_irqsave(host-lock, flags);
+ present = host-ops-get_cd(host);
+ spin_unlock_irqrestore(host-lock, flags);
+ }
+
+ return present;
+}
+
I think this function has to take care of standard sd host behaviour: if
a specific host has implemented the get_cd callback, then use it; if
not, then see the BROKEN_DETECT quirk; finally, use the present register
to get the value like the sdhci_request function does.
-Aaron
static void sdhci_enable_sdio_irq_nolock(struct sdhci_host *host, int
enable)
{
if (host-flags SDHCI_DEVICE_DEAD)
@@ -1884,6 +1905,7 @@ static const struct mmc_host_ops sdhci_ops = {
.request= sdhci_request,
.set_ios= sdhci_set_ios,
.get_ro = sdhci_get_ro,
+ .get_cd = sdhci_get_cd,
.hw_reset = sdhci_hw_reset,
.enable_sdio_irq = sdhci_enable_sdio_irq,
.start_signal_voltage_switch= sdhci_start_signal_voltage_switch,
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 0a5b654..82f4d27 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -69,6 +69,7 @@
#define SDHCI_SPACE_AVAILABLE 0x0400
#define SDHCI_DATA_AVAILABLE 0x0800
#define SDHCI_CARD_PRESENT0x0001
+#define SDHCI_CARD_CDPL 0x0004
#define SDHCI_WRITE_PROTECT 0x0008
#define SDHCI_DATA_LVL_MASK 0x00F0
#define SDHCI_DATA_LVL_SHIFT 20
@@ -261,6 +262,7 @@ struct sdhci_ops {
void(*set_clock)(struct sdhci_host *host, unsigned int clock);
+ int (*get_cd)(struct sdhci_host *host);
int (*enable_dma)(struct sdhci_host *host);
unsigned int(*get_max_clock)(struct sdhci_host *host);
unsigned int(*get_min_clock)(struct sdhci_host *host);
--
1.7.5.4
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4 v5] SDHCI: add sdhci_get_cd callback to detect the card
On Fri, Jan 13, 2012 at 04:50:11AM +, Huang Changming-R66093 wrote: I think this function has to take care of standard sd host behaviour: if a specific host has implemented the get_cd callback, then use it; if not, then see the BROKEN_DETECT quirk; finally, use the present register to get the value like the sdhci_request function does. Yes, I have thought about it. In this patch, only FSL eSDHC use it. Only FSL eSDHC defines it, but all sd hosts will use it since the call is made in core.c with host-ops-get_cd. If you only cares about FSL eSDHC, then probably you should do this in sdhci.c instead of core.c, by adding a callback to the sdhci_ops and in the sdhci_request call, instead of checking present register, call the get_cd callback if available. For the other platform, the get_cd is NULL. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
Hi,
On Fri, Jan 13, 2012 at 04:52:42AM +, Huang Changming-R66093 wrote:
For sd hosts, this should only happen for hosts which have
SDHCI_QUIRK_BROKEN_CARD_DETECTION set.
Yes, but which will impact the performance.
You only set this bit when your host broke, and if your host has other
means to detect this, then go with your newly added callback.
Therefore, add callback function get_cd() to check whether the card
has been removed when the driver has this callback function.
I don't quite get the meaning of cd, what does get_cd suppose to mean?
This get_cd callback is implement in the special platform, because some SOC
don't support this feature
If the card is present, 1 will return, if the card is absent, 0 will
return.
If the controller will not support this feature, -ENOSYS will return.
What about get_present, return 0 for present, and return error code
otherwise like the alive function does.
The hook to detect card is get_cd in the kernel, don't need the new.
I didn't mean to add a new function, I just can't get the meaning of cd.
I just did a grep of the code and find some same fuction names in
various host files, I think it's OK to continue with this name.
Signed-off-by: Jerry Huang chang-ming.hu...@freescale.com
CC: Chris Ball c...@laptop.org
---
changes for v2:
- when controller don't support get_cd, return -ENOSYS
- add the CC
changes for v3:
- enalbe the controller clock in platform, instead of core
changes for v4:
- move the detect code to core.c according to the new structure
drivers/mmc/core/core.c | 10 --
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
6db6621..d570c72 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2060,7 +2060,7 @@ static int mmc_rescan_try_freq(struct mmc_host
*host, unsigned freq)
snip
int _mmc_detect_card_removed(struct mmc_host *host) {
- int ret;
+ int ret = -ENOSYS;
if ((host-caps MMC_CAP_NONREMOVABLE) ||
!host-bus_ops-alive)
return 0;
@@ -2068,7 +2068,13 @@ int _mmc_detect_card_removed(struct mmc_host
*host)
if (!host-card || mmc_card_removed(host-card))
return 1;
- ret = host-bus_ops-alive(host);
+ if (host-ops-get_cd) {
+ ret = host-ops-get_cd(host);
+ if (ret = 0)
+ ret = !ret;
+ }
+ if (ret 0)
+ ret = host-bus_ops-alive(host);
if (ret) {
mmc_card_set_removed(host-card);
pr_debug(%s: card remove detected\n,
mmc_hostname(host));
--
1.7.5.4
And the code can be changed to something like:
if (host-ops-get_present)
ret = host-ops-get_present(host);
else
ret = host-bus_ops-alive(host);
if (ret) {
mmc_card_set_removed(host-card);
pr_debug(%s: card remove detected\n, mmc_hostname(host));
}
Does this make sense?
No.
Because the get_cd is the hook to detect card in mmc_host_ops structure in
include/linux/mmc/host.h.
We don't need to add new.
I just suggested to change the name and use a different return value for
this get_cd function, not to add a new function call.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4 v3] MMC/core: Add f_min to mmc_power_on()
Hi, On Fri, Jan 13, 2012 at 03:39:53AM +, Huang Changming-R66093 wrote: -Original Message- From: Aaron Lu [mailto:aaron...@amd.com] Sent: Friday, January 13, 2012 11:27 AM To: Huang Changming-R66093 Cc: linux-mmc@vger.kernel.org; Chris Ball Subject: Re: [PATCH 1/4 v3] MMC/core: Add f_min to mmc_power_on() Hi, On Fri, Jan 13, 2012 at 02:24:46AM +, Huang Changming-R66093 wrote: Hi, Chris, Could you have any comment about this patch? Can it go into 3.3 or 3.4? -Original Message- From: Huang Changming-R66093 Sent: Friday, December 09, 2011 10:54 AM To: linux-mmc@vger.kernel.org Cc: Huang Changming-R66093; Chris Ball Subject: [PATCH 1/4 v3] MMC/core: Add f_min to mmc_power_on() From: Jerry Huang chang-ming.hu...@freescale.com When f_init is zero, the SDHC can't work correctly. So f_min will replace f_init, when f_init is zero. What about setting f_init before your call to mmc_power_up? f_init is initialized only by mmc_rescan_try_freq, and mmc_power_up will use it. But mmc_power_up will called not only by mmc_rescan_try_freq, but also by other functions. You will see it in my previous email about it. Yes, I know there are other callers for mmc_power_up. Either you set host-f_init in mmc_power_up, or you set it before you call mmc_power_up. I prefer the latter one but I'm not sure which one should be used. Maybe Chris can comment on this. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
On Fri, Jan 13, 2012 at 07:05:19AM +, Huang Changming-R66093 wrote: If you read the previous email about this serial patches discussed with other guys, you can understand. -Original Message- From: Aaron Lu [mailto:aaron...@amd.com] Sent: Friday, January 13, 2012 2:28 PM To: Huang Changming-R66093 Cc: linux-mmc@vger.kernel.org; Chris Ball Subject: Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card Hi, On Fri, Jan 13, 2012 at 04:52:42AM +, Huang Changming-R66093 wrote: For sd hosts, this should only happen for hosts which have SDHCI_QUIRK_BROKEN_CARD_DETECTION set. Yes, but which will impact the performance. You only set this bit when your host broke, and if your host has other means to detect this, then go with your newly added callback. To detect the card state, if SDHCI_QUIRK_BROKEN_CARD_DETECTION is set, then driver will send command to card, which will cause the bad performance. So my patch will do: 1. use get_cd to detect the card state, if platform don't support this feature(get_cd is not defined in special platform), -ENOSYS will be returned 2. if -ENOSYS, then send the command to card. OK, this is clear. I didn't know there is a get_cd callback already defined in sdhci_ops with a clear return value description, sorry for the mess. I just suggested to change the name and use a different return value for this get_cd function, not to add a new function call. The description for get_cd in file include/linux/mmc/host.h: * Return values for the get_cd callback should be: * 0 for a absent card * 1 for a present card * -ENOSYS when not supported (equal to NULL callback) * or a negative errno value when something bad happened I don't think your suggest is reasonable. Agree. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: typo in Documentation/devices.txt ?
Hi,
On Thu, Jan 05, 2012 at 07:08:27AM +, 허종만 wrote:
Hi, all..
Here is description for MMC block devices in Documentation/devices.txt :
179 block MMC block devices
0 = /dev/mmcblk0 First SD/MMC card
1 = /dev/mmcblk0p1First partition on first MMC card
8 = /dev/mmcblk1 Second SD/MMC card
^^
The start of next SD/MMC card can be configured with
CONFIG_MMC_BLOCK_MINORS, or overridden at boot/modprobe
time using the mmcblk.perdev_minors option. That would
bump the offset between each card to be the configured
value instead of the default 8.
Shouldn't 8 = /dev/mmcblk1 be 9 = /dev/mmcblk1 ?
No, mmcblk1's minor is 8, mmcblk2's minor is 16, etc...
If we have 8 partitions on first MMC card, we need mmcblk0p1 ~ mmcblk0p8
devices, with minor number 1 ~ 8.
Am I misunderstanding?
Each mmc block device by default has 8 minors, not 8 partitions. Minor 0
represent the whole disk. So each mmc block device at most has 7
partitions instead of 8.
See code in function mmc_blk_alloc_req on assigning minor value to the
whole disk of the block device:
md-disk-first_minor = devidx * perdev_minors;
And code in function blk_alloc_devt on checking partition number against
disk minors:
/* in consecutive minor range? */
if (part-partno disk-minors) {
*devt = MKDEV(disk-major, disk-first_minor + part-partno);
return 0;
}
-Aaron
Regards,
Jongman Heo.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sdhci max_blk_size
On Wed, Jan 04, 2012 at 04:01:07PM +0100, Matthieu CASTET wrote:
Hi,
our controller set Max Block Length to 3 (4096 byte), but the linux driver
ignore this value [1].
Is there any reason to do that .
The reason is, sd host controller spec defines 3 as reserved, not 4096.
Or it is code that was written with an older sdhci spec, and we can ignore
this
check today.
The current sdhci driver follows v3 spec, is the v4 spec out that
defined this bit?
Matthieu
PS : please keep me in CC.
[1]
if (mmc-max_blk_size = 3) {
pr_warning(%s: Invalid maximum block size,
assuming 512 bytes\n, mmc_hostname(mmc));
mmc-max_blk_size = 0;
}
--
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: sdhci: deactivate tuning timer when host is suspending
On Mon, Jan 02, 2012 at 07:02:27PM -0500, Chris Ball wrote:
Hi,
On Mon, Dec 26 2011, Aaron Lu wrote:
From: Philip Rakity prak...@marvell.com
Since we are suspending, the tuning timer should be deactivated.
Signed-off-by: Philip Rakity prak...@marvell.com
Acked-by: Aaron Lu aaron...@amd.com
---
drivers/mmc/host/sdhci.c |3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index ab6018f..1abbd26 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2347,8 +2347,7 @@ int sdhci_suspend_host(struct sdhci_host *host)
if (host-version = SDHCI_SPEC_300 host-tuning_count
host-tuning_mode == SDHCI_TUNING_MODE_1) {
host-flags = ~SDHCI_NEEDS_RETUNING;
- mod_timer(host-tuning_timer, jiffies +
- host-tuning_count * HZ);
+ del_timer_sync(host-tuning_timer);
}
ret = mmc_suspend_host(host-mmc);
Pushed to mmc-next for 3.3 with a stable@ tag, thanks.
I'm sorry Chris, this patch is deprecated and please apply the following
patch with Adrian's ack instead:
[PATCH 1/2] mmc: sdhci: Fix tuning timer incorrect setting when suspending host
Thanks.
- Chris.
--
Chris Ball c...@laptop.org 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/2] mmc: sdhci: Deal with failure case in sdhci_suspend_host
V3: Rework on top of Adrian's vmmc patch
V2: Error processing code in sdhci_pci_suspend should not be deleted,
it is used to resume hosts which are already successfully suspended for
a multi slot pci device as suggested by Adrian.
If there are errors happened in sdhci_suspend_host, handle it so that
when the function returns with an error, the host's behaviour is the
same before this function call, e.g. card detection is enabled and
tuning timer is active, etc.
Signed-off-by: Philip Rakity prak...@marvell.com
Signed-off-by: Aaron Lu aaron...@amd.com
Cc: Adrian Hunter adrian.hun...@intel.com
---
drivers/mmc/host/sdhci.c | 18 ++
1 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 988ae06..823b83e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2349,20 +2349,30 @@ out:
int sdhci_suspend_host(struct sdhci_host *host)
{
- int ret;
+ int ret, has_tuning_timer;
sdhci_disable_card_detection(host);
/* Disable tuning since we are suspending */
- if (host-version = SDHCI_SPEC_300 host-tuning_count
- host-tuning_mode == SDHCI_TUNING_MODE_1) {
+ has_tuning_timer = host-version = SDHCI_SPEC_300
+ host-tuning_count host-tuning_mode == SDHCI_TUNING_MODE_1;
+ if (has_tuning_timer) {
del_timer_sync(host-tuning_timer);
host-flags = ~SDHCI_NEEDS_RETUNING;
}
ret = mmc_suspend_host(host-mmc);
- if (ret)
+ if (ret) {
+ if (has_tuning_timer) {
+ host-flags |= SDHCI_NEEDS_RETUNING;
+ mod_timer(host-tuning_timer, jiffies +
+ host-tuning_count * HZ);
+ }
+
+ sdhci_enable_card_detection(host);
+
return ret;
+ }
free_irq(host-irq, host);
--
1.7.7.5
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mmc: sdhci: Deal with failure case in sdhci_suspend_host
On Wed, Dec 28, 2011 at 02:23:32PM +0200, Adrian Hunter wrote: - -err_pci_suspend: - while (--i = 0) - sdhci_resume_host(chip-slots[i]-host); - return ret; This doesn't look right. This is about having multiple host controllers on the same PCI device. If those hosts have been successfully suspended, then they must be resumed on error. You are right, I failed to understand the code. Will send the patch again without touching the pci recover code, thanks. } -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] mmc: sdhci: Deal with failure case in sdhci_suspend_host
V2: Error processing code in sdhci_pci_suspend should not be deleted,
it is used to resume hosts which are already successfully suspended for
a multi slot pci device as suggested by Adrian.
If there are errors happened in sdhci_suspend_host, handle it so that
when the function returns with an error, the host's behaviour is the
same before this function call, e.g. card detection is enabled and
tuning timer is active, etc.
Signed-off-by: Philip Rakity prak...@marvell.com
Signed-off-by: Aaron Lu aaron...@amd.com
Cc: Adrian Hunter adrian.hun...@intel.com
---
drivers/mmc/host/sdhci.c | 27 +--
1 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 2007d37..37aeb81 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2339,25 +2339,40 @@ out:
int sdhci_suspend_host(struct sdhci_host *host)
{
- int ret;
+ int ret, has_tuning_timer;
sdhci_disable_card_detection(host);
/* Disable tuning since we are suspending */
- if (host-version = SDHCI_SPEC_300 host-tuning_count
- host-tuning_mode == SDHCI_TUNING_MODE_1) {
+ has_tuning_timer = host-version = SDHCI_SPEC_300
+ host-tuning_count host-tuning_mode == SDHCI_TUNING_MODE_1;
+ if (has_tuning_timer) {
del_timer_sync(host-tuning_timer);
host-flags = ~SDHCI_NEEDS_RETUNING;
}
ret = mmc_suspend_host(host-mmc);
if (ret)
- return ret;
+ goto err_suspend;
+
+ if (host-vmmc) {
+ ret = regulator_disable(host-vmmc);
+ if (ret)
+ goto err_suspend;
+ }
free_irq(host-irq, host);
- if (host-vmmc)
- ret = regulator_disable(host-vmmc);
+ return 0;
+
+err_suspend:
+ if (has_tuning_timer) {
+ host-flags |= SDHCI_NEEDS_RETUNING;
+ mod_timer(host-tuning_timer, jiffies +
+ host-tuning_count * HZ);
+ }
+
+ sdhci_enable_card_detection(host);
return ret;
}
--
1.7.7.4
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] mmc: sdhci: Fix for sdhci suspend function
Fixed a bug in sdhci_suspend_host, the tuning timer should be deactivated instead of reactivated when suspending. Handle failure case in sdhci_suspend_host, so that calling function does not need to take care of the error recovery. mmc: sdhci: Fix tuning timer incorrect setting when suspending host mmc: sdhci: Deal with failure case in sdhci_suspend_host drivers/mmc/host/sdhci-pci.c |9 ++--- drivers/mmc/host/sdhci.c | 30 ++ 2 files changed, 24 insertions(+), 15 deletions(-) -- 1.7.7.4 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mmc: sdhci: deactivate tuning timer when host is suspending
From: Philip Rakity prak...@marvell.com
Since we are suspending, the tuning timer should be deactivated.
Signed-off-by: Philip Rakity prak...@marvell.com
Acked-by: Aaron Lu aaron...@amd.com
---
drivers/mmc/host/sdhci.c |3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index ab6018f..1abbd26 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2347,8 +2347,7 @@ int sdhci_suspend_host(struct sdhci_host *host)
if (host-version = SDHCI_SPEC_300 host-tuning_count
host-tuning_mode == SDHCI_TUNING_MODE_1) {
host-flags = ~SDHCI_NEEDS_RETUNING;
- mod_timer(host-tuning_timer, jiffies +
- host-tuning_count * HZ);
+ del_timer_sync(host-tuning_timer);
}
ret = mmc_suspend_host(host-mmc);
--
1.7.7.4
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] SD/MMC: fix the issue of SDHC performance regression
On Mon, Nov 07, 2011 at 08:20:10AM -0500, Chris Ball wrote:
Have you seen:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=f2815f68dabbb373fd1c9f0fd4a609d486697c2b
(mmc: sd: Handle SD3.0 cards not supporting UHS-I bus speed mode)
which is already in mainline? I think your patch is identical.
Hi Chris,
I think the existing code is somewhat confusing, since SDR50 means
100MHZ frequency while high speed is 50MHZ.
The reason it is correct is UHS_SDR50_BUS_SPEED is defined as 2,
which happened to be the same value as (1 UHS_SDR25_BUS_SPEED).
How about change it like this:
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index a230e7f..670fd7f 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -306,7 +306,7 @@ static int mmc_read_switch(struct mmc_card *card)
goto out;
}
- if (status[13] UHS_SDR50_BUS_SPEED)
+ if (status[13] SD_MODE_UHS_SDR25)
card-sw_caps.hs_max_dtr = 5000;
if (card-scr.sda_spec3) {
SDR25 is also 50MHZ, the same frequency as high speed.
Or we can add a new macro for high speed like qiang has done,
which one you prefer?
Thanks,
Aaron
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V1] mmc: core: HS200 mode support for eMMC 4.5
On Thu, Sep 22, 2011 at 04:43:40PM +0530, Girish K S wrote: This patch adds the support of the HS200 bus speed for eMMC 4.5 devices. The eMMC 4.5 devices have support for 200MHz bus speed. The mmc core and host modules have been touched to add support for this module. It is necessary to know the card type in the sdhci.c file to add support for eMMC tuning function. So card.h file is included to import the card data structure. Signed-off-by: Girish K S girish.shivananja...@linaro.org --- v1: cases which produce same result have been combined to reduce repeated assignments. patch recreated after rebase to chris balls mmc-next branch. Your patch does not apply to mmc-next tree... @@ -1661,7 +1663,10 @@ static int sdhci_execute_tuning(struct mmc_host *mmc) if (!tuning_loop_counter !timeout) break; The sdhci_execute_tuning will not proceed here unless the ctrl register indicates the host is running at UHS_SDR104 or UHS_SDR50 with SDHCI_SDR50_NEEDS_TUNING flag set. Does your host also reflect this when running at 200Mhz bus speed? - cmd.opcode = MMC_SEND_TUNING_BLOCK; + if (mmc-card-type == MMC_TYPE_MMC) + cmd.opcode = MMC_SEND_TUNING_BLOCK_HS200; + else + cmd.opcode = MMC_SEND_TUNING_BLOCK; cmd.arg = 0; cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC; cmd.retries = 0; -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: core: Set correct bus mode before card init
Hi Ulf, I'm not familiar with mmc, but I've some questions on sd below. On Thu, Sep 15, 2011 at 05:50:38PM +0200, Ulf Hansson wrote: Earlier all cards where initiated with bus mode set as OPENDRAIN, and then later switched to PUSHPULL. According to the MMC/SD/SDIO specifications only MMC cards use OPENDRAIN during init. For both SD and SDIO the bus mode shall be PUSHPULL before attempting to init the card. AFAIK, there is no open drain mode in sd, and the sd host controller actually does not care about this setting(sdhci_set_ios does not manipulate bus_mode stored in ios). The consequence of having incorrect bus mode can lead to not being able to detect the card. Therefore the default behavior have now been changed to PUSHPULL in mmc_power_up, and will only be temporarily switched when trying to attach or init a MMC card. Do you see any sd cards that failed to be detected due to the incorrect bus_mode setting? Anyway, your patch removed an unnecessary call to mmc_set_bus_mode which should have no effect if the underlying controller is sdhci. -Aaron -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: confusion regarding the CMD19 and CMD21 in eMMC/SD card spec
On Fri, Sep 16, 2011 at 11:00:12AM +0530, Girish K S wrote: Hello Aaron Lu, Hi, please check the mmc 4.5 specification CMD21 is tuning command. So you are using a mmc card with a sd host? -Aaron -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mmc: core: add sd uhs string for mmc_ios_show
This is a minor fix. It makes mmc_ios_show print proper string when the host's timing is one of the newly added UHS-I modes. Signed-off-by: Aaron Lu aaron...@amd.com --- drivers/mmc/core/debugfs.c |9 + 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c index 5acd707..96c603b 100644 --- a/drivers/mmc/core/debugfs.c +++ b/drivers/mmc/core/debugfs.c @@ -114,6 +114,15 @@ static int mmc_ios_show(struct seq_file *s, void *data) case MMC_TIMING_SD_HS: str = sd high-speed; break; + case MMC_TIMING_UHS_SDR50: + str = sd uhs SDR50; + break; + case MMC_TIMING_UHS_SDR104: + str = sd uhs SDR104; + break; + case MMC_TIMING_UHS_DDR50: + str = sd uhs DDR50; + break; default: str = invalid; break; -- 1.7.4.4 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mmc: sdhci: add support for re-tuning mode 2
Hosts which support re-tuning mode 2 has the capability to indicate
the re-tuning timing by issuing re-tuning request during data transfers.
During non-data transfers, re-tuning timing is determined by either
re-tuning timer or re-tuning request.
Since there is no way to determine the host's capability to generate
re-tuning request during non-data transfers, we will start the re-tuning
timer by default for re-tuning mode 2. If we ever received a re-tuning
request during non-data transfers, that means the host is also capable
of generating re-tuning request for non-data transfers, we will then
deactivate the re-tuning timer altogether.
The SDHCI_RETUNING_TIMER flag is added to indicate the fact that the
host requires re-tuning timer to trigger the re-tuning timing.
The SDHCI_NEEDS_RETUNING, SDHCI_RETUNING_TIMER flags and the max block
count of the host will be restored to their default values since they
are affected by different cards inserted too.
Signed-off-by: Aaron Lu aaron...@amd.com
---
drivers/mmc/host/sdhci.c | 128 +
drivers/mmc/host/sdhci.h |3 +
include/linux/mmc/sdhci.h |4 ++
3 files changed, 102 insertions(+), 33 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0e02cc1..f620c72 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -222,6 +222,22 @@ static void sdhci_reinit(struct sdhci_host *host)
{
sdhci_init(host, 0);
sdhci_enable_card_detection(host);
+
+ if (host-version = SDHCI_SPEC_300) {
+ /*
+* Clear re-tuning related flags, since these flags
+* are also affected by different cards inserted
+*/
+ host-flags = ~(SDHCI_NEEDS_RETUNING |
+ SDHCI_RETUNING_TIMER);
+
+ /*
+* restore max block count, since it might be
+* reduced due to re-tuning mode 1 and 2
+*/
+ host-mmc-max_blk_count =
+ (host-quirks SDHCI_QUIRK_NO_MULTIBLOCK) ? 1 : 65535;
+ }
}
static void sdhci_activate_led(struct sdhci_host *host)
@@ -1245,7 +1261,7 @@ static void sdhci_request(struct mmc_host *mmc, struct
mmc_request *mrq)
present_state = sdhci_readl(host, SDHCI_PRESENT_STATE);
/*
-* Check if the re-tuning timer has already expired and there
+* Check if the re-tuning timing has already arrived and there
* is no on-going data transfer. If so, we need to execute
* tuning procedure before sending command.
*/
@@ -1733,34 +1749,41 @@ static int sdhci_execute_tuning(struct mmc_host *mmc)
out:
/*
-* If this is the very first time we are here, we start the retuning
-* timer. Since only during the first time, SDHCI_NEEDS_RETUNING
-* flag won't be set, we check this condition before actually starting
-* the timer.
+* If this is the very first time we are here, we setup the
+* corresponding re-tuning condition. Since only during the
+* first time, SDHCI_NEEDS_RETUNING flag won't be set.
*/
- if (!(host-flags SDHCI_NEEDS_RETUNING) host-tuning_count
- (host-tuning_mode == SDHCI_TUNING_MODE_1)) {
- mod_timer(host-tuning_timer, jiffies +
- host-tuning_count * HZ);
- /* Tuning mode 1 limits the maximum data length to 4MB */
- mmc-max_blk_count = (4 * 1024 * 1024) / mmc-max_blk_size;
+ if (!(host-flags SDHCI_NEEDS_RETUNING)) {
+ if (host-tuning_count
+ host-tuning_mode != SDHCI_TUNING_MODE_RSV) {
+ host-flags |= SDHCI_RETUNING_TIMER;
+ mod_timer(host-tuning_timer, jiffies +
+ host-tuning_count * HZ);
+ }
+ /* Enable re-tuning event for tuning mode 2 */
+ if (host-tuning_mode == SDHCI_TUNING_MODE_2)
+ ier |= SDHCI_INT_RETUNING;
+ /* Tuning mode 1 and 2 limits the maximum data length to 4MB */
+ if (host-tuning_mode == SDHCI_TUNING_MODE_1 ||
+ host-tuning_mode == SDHCI_TUNING_MODE_2)
+ mmc-max_blk_count = (4 * 1024 * 1024) /
+ mmc-max_blk_size;
} else {
host-flags = ~SDHCI_NEEDS_RETUNING;
- /* Reload the new initial value for timer */
- if (host-tuning_mode == SDHCI_TUNING_MODE_1)
+ /* Reload the new initial value for re-tuning timer */
+ if (host-flags SDHCI_RETUNING_TIMER)
mod_timer(host-tuning_timer, jiffies +
host-tuning_count * HZ);
}
/*
* In case tuning fails, host
Re: [PATCH] mmc: sdhci: fix retuning timer wrongly deleted in sdhci_tasklet_finish
On Thu, Jul 21, 2011 at 05:35:02PM +0800, zhangfei gao wrote: Does the execute_tuning is called again? del_timer is not delete timer really, but deactivate the timer, which could be re-activated by mod_timer. So if execute_tuning is called, the mod_timer will tigger the tuning timer again. Hi zhangfei, Thanks for the comment. The execute_tuning will be called at two places: 1 In mmc_sd_init_uhs_card, when host is initializing an UHS card, and the re-tuning timer will be activated for the first time; 2 When re-tuning timer expired So if the re-tuning timer is deactivated in sdhci_tasklet_finish, execute_tuning will have no chance of getting called again, and the host will not be able to do the re-tuning anymore. Signed-off-by: Aaron Lu aaron...@amd.com --- drivers/mmc/host/sdhci.c | 3 --- 1 files changed, 0 insertions(+), 3 deletions(-) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 91d9892..6250bac 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1863,9 +1863,6 @@ static void sdhci_tasklet_finish(unsigned long param) del_timer(host-timer); - if (host-version = SDHCI_SPEC_300) - del_timer(host-tuning_timer); - mrq = host-mrq; /* -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: sdhci: fix retuning timer wrongly deleted in sdhci_tasklet_finish
Hi Philip,
On Thu, Jul 21, 2011 at 01:27:11PM -0700, Philip Rakity wrote:
Aaron,
Code is fine.
Thanks.
Do you want to also fix the problem with suspend or should I do the patch ?
When we are suspending we should kill the tuning timer.
Yes, that is a problem.
I've another patch to add support for re-tuning mode 2 queued for
submit which should have fixed this problem, but I didn't touch anything
if mmc_suspend_host failed, so when I submit the other patch, please
take you time to review it and let's see how we can fix the problem
altogether :-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 4da6a4d..88c25e8 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2289,18 +2289,14 @@ int sdhci_suspend_host(struct sdhci_host *host,
pm_message_t state)
sdhci_disable_card_detection(host);
- /* Disable tuning since we are suspending */
- if (host-version = SDHCI_SPEC_300 host-tuning_count
- host-tuning_mode == SDHCI_TUNING_MODE_1) {
- host-flags = ~SDHCI_NEEDS_RETUNING;
- mod_timer(host-tuning_timer, jiffies +
- host-tuning_count * HZ);
- }
-
ret = mmc_suspend_host(host-mmc);
if (ret)
return ret;
+ /* Disable tuning since we are suspending */
+ if (host-version = SDHCI_SPEC_300)
+ del_timer_sync(host-tuning_timer);
+
free_irq(host-irq, host);
if (host-vmmc)
Philip
On Jul 21, 2011, at 3:03 AM, Aaron Lu wrote:
On Thu, Jul 21, 2011 at 05:35:02PM +0800, zhangfei gao wrote:
Does the execute_tuning is called again?
del_timer is not delete timer really, but deactivate the timer, which
could be re-activated by mod_timer.
So if execute_tuning is called, the mod_timer will tigger the tuning
timer again.
Hi zhangfei,
Thanks for the comment.
The execute_tuning will be called at two places:
1 In mmc_sd_init_uhs_card, when host is initializing an UHS card,
and the re-tuning timer will be activated for the first time;
2 When re-tuning timer expired
So if the re-tuning timer is deactivated in sdhci_tasklet_finish,
execute_tuning will have no chance of getting called again, and the
host will not be able to do the re-tuning anymore.
Signed-off-by: Aaron Lu aaron...@amd.com
---
drivers/mmc/host/sdhci.c |3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 91d9892..6250bac 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1863,9 +1863,6 @@ static void sdhci_tasklet_finish(unsigned long
param)
del_timer(host-timer);
- if (host-version = SDHCI_SPEC_300)
- del_timer(host-tuning_timer);
-
mrq = host-mrq;
/*
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mmc: sdhci: fix retuning timer wrongly deleted in sdhci_tasklet_finish
Currently, the retuning timer for retuning mode 1 will be deleted in function sdhci_tasklet_finish after a mmc request done, which will make retuning timing never trigger again. This patch fixed this problem. Signed-off-by: Aaron Lu aaron...@amd.com --- drivers/mmc/host/sdhci.c |3 --- 1 files changed, 0 insertions(+), 3 deletions(-) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 91d9892..6250bac 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1863,9 +1863,6 @@ static void sdhci_tasklet_finish(unsigned long param) del_timer(host-timer); - if (host-version = SDHCI_SPEC_300) - del_timer(host-tuning_timer); - mrq = host-mrq; /* -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
