Re: [PATCH v8 1/2] mmc: add packed command feature of eMMC4.5
On Mon, July 9, 2012 10:54 pm, Chris Ball wrote:
Hi,
On Fri, Jun 29 2012, Seungwon Jeon wrote:
This patch adds packed command feature of eMMC4.5.
The maximum number for packing read(or write) is offered
and exception event relevant to packed command which is
used for error handling is enabled. If host wants to use
this feature, MMC_CAP2_PACKED_CMD should be set.
Signed-off-by: Seungwon Jeon tgih@samsung.com
Reviewed-by: Maya Erez me...@codeaurora.org
Reviewed-by: Subhash Jadavani subha...@codeaurora.org
Reviewed-by: Namjae Jeon linkinj...@gmail.com
---
drivers/mmc/core/mmc.c | 25 +
include/linux/mmc/card.h |3 +++
include/linux/mmc/host.h |4
include/linux/mmc/mmc.h | 15 +++
4 files changed, 47 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 258b203..bfb271f 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -516,6 +516,11 @@ static int mmc_read_ext_csd(struct mmc_card *card,
u8 *ext_csd)
} else {
card-ext_csd.data_tag_unit_size = 0;
}
+
+card-ext_csd.max_packed_writes =
+ext_csd[EXT_CSD_MAX_PACKED_WRITES];
+card-ext_csd.max_packed_reads =
+ext_csd[EXT_CSD_MAX_PACKED_READS];
} else {
card-ext_csd.data_sector_size = 512;
}
@@ -1246,6 +1251,26 @@ static int mmc_init_card(struct mmc_host *host,
u32 ocr,
}
}
+if ((host-caps2 MMC_CAP2_PACKED_WR
+card-ext_csd.max_packed_writes 0) ||
+(host-caps2 MMC_CAP2_PACKED_RD
+card-ext_csd.max_packed_reads 0)) {
+err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+EXT_CSD_EXP_EVENTS_CTRL,
+EXT_CSD_PACKED_EVENT_EN,
+card-ext_csd.generic_cmd6_time);
Sorry, I don't have a copy of the eMMC 4.5 spec -- is PACKED_EVENT_EN
a one-time programmable fuse on the eMMC, like BKOPS_ENABLE was?
Thanks,
- Chris.
--
Chris Ball c...@laptop.org http://printf.net/
One Laptop Per Child
No, it's not.
--
Sent by consultant of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of 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 v8 1/2] mmc: add packed command feature of eMMC4.5
On Tue, Jul 10, 2012 at 11:24 AM, Chris Ball c...@laptop.org wrote:
Hi,
On Fri, Jun 29 2012, Seungwon Jeon wrote:
This patch adds packed command feature of eMMC4.5.
The maximum number for packing read(or write) is offered
and exception event relevant to packed command which is
used for error handling is enabled. If host wants to use
this feature, MMC_CAP2_PACKED_CMD should be set.
Signed-off-by: Seungwon Jeon tgih@samsung.com
Reviewed-by: Maya Erez me...@codeaurora.org
Reviewed-by: Subhash Jadavani subha...@codeaurora.org
Reviewed-by: Namjae Jeon linkinj...@gmail.com
---
drivers/mmc/core/mmc.c | 25 +
include/linux/mmc/card.h |3 +++
include/linux/mmc/host.h |4
include/linux/mmc/mmc.h | 15 +++
4 files changed, 47 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 258b203..bfb271f 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -516,6 +516,11 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8
*ext_csd)
} else {
card-ext_csd.data_tag_unit_size = 0;
}
+
+ card-ext_csd.max_packed_writes =
+ ext_csd[EXT_CSD_MAX_PACKED_WRITES];
+ card-ext_csd.max_packed_reads =
+ ext_csd[EXT_CSD_MAX_PACKED_READS];
} else {
card-ext_csd.data_sector_size = 512;
}
@@ -1246,6 +1251,26 @@ static int mmc_init_card(struct mmc_host *host, u32
ocr,
}
}
+ if ((host-caps2 MMC_CAP2_PACKED_WR
+ card-ext_csd.max_packed_writes 0) ||
+ (host-caps2 MMC_CAP2_PACKED_RD
+ card-ext_csd.max_packed_reads 0)) {
+ err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+ EXT_CSD_EXP_EVENTS_CTRL,
+ EXT_CSD_PACKED_EVENT_EN,
+ card-ext_csd.generic_cmd6_time);
Sorry, I don't have a copy of the eMMC 4.5 spec -- is PACKED_EVENT_EN
a one-time programmable fuse on the eMMC, like BKOPS_ENABLE was?
It's not. PACKED_EVENT_EN is a bit set into EXCEPTION_EVENTS_CTRL
register, which is reset duing a power cycle
--
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 v8 1/2] mmc: add packed command feature of eMMC4.5
Hi, On Tue, Jul 10 2012, me...@codeaurora.org wrote: Sorry, I don't have a copy of the eMMC 4.5 spec -- is PACKED_EVENT_EN a one-time programmable fuse on the eMMC, like BKOPS_ENABLE was? No, it's not. Excellent, thanks. I've pushed both v8 patches to mmc-next. Before pushing it up to Linus, I'd like to get a better idea of the variety of performances differences seen using packed writes -- has it been tried on devices from multiple eMMC vendors, and multiple host controller vendors? Can we try to build up a quick table of results, with a standard test like iozone, to check that we've got decent test coverage? Seungwon suggests using: iozone -az -i0 -I -s 10m -f /target/test -e I'm also worried about how much code this is adding, but I agree that it's worth it if the performance benefits are as described. 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] 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 v8 1/2] mmc: add packed command feature of eMMC4.5
On Tue, July 10, 2012 12:33 am, Chris Ball wrote: Hi, On Tue, Jul 10 2012, me...@codeaurora.org wrote: Sorry, I don't have a copy of the eMMC 4.5 spec -- is PACKED_EVENT_EN a one-time programmable fuse on the eMMC, like BKOPS_ENABLE was? No, it's not. Excellent, thanks. I've pushed both v8 patches to mmc-next. Before pushing it up to Linus, I'd like to get a better idea of the variety of performances differences seen using packed writes -- has it been tried on devices from multiple eMMC vendors, and multiple host controller vendors? Can we try to build up a quick table of results, with a standard test like iozone, to check that we've got decent test coverage? Seungwon suggests using: iozone -az -i0 -I -s 10m -f /target/test -e I'm also worried about how much code this is adding, but I agree that it's worth it if the performance benefits are as described. Thanks, - Chris. -- Chris Ball c...@laptop.org http://printf.net/ One Laptop Per Child -- Hi Chris, I suggest to also use lmdd as a benchmark test for write packing, since write packing is mostly beneficial in long sequential operations. It can be used as follows: lmdd if=internal of=/data/file1 bs=128k count=3000 Thanks, Maya -- Sent by consultant of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of 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 5/5] mmc: card: Add RPMB support in IOCTL interface
Repost in text format
2012/7/10 Namjae Jeon linkinj...@gmail.com
+ err = mmc_blk_part_switch(card, md);
+ if (err)
+ goto cmd_rel_host;
+
Should it wrapped by if (is_rpbm) condition ? It can be called in other
ioctl.
It was my first implementation, but in that case you have to manage a
session, switching on RPMB partition at the begining of the IOCTL and
restore previous partition at the end. (because RPMB partition doesn't
support all commands).
Having a look to how mmc driver manages partition selection, I saw
that mmc_blk_issue_rq for instance calls mmc_blk_part_switch in the
beginning to be sure it's on the right partition, but never restores
The proposal is to use same way and to make it compliant with Boot and
GP partitions if there are any special commands supported by them.
if (idata-ic.is_acmd) {
err = mmc_app_cmd(card-host, card);
if (err)
goto cmd_rel_host;
}
--
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
mmc: mxs: DEADLOCK
Hi,
I was able to get deadlock with CONFIG_DEBUG_SPINLOCK enabled. I added
also CONFIG_PROVE_LOCKING to get more verbose output. I got following
error message after SDIO device has been powered.
I'm able to replicate issue with Linux next-20120710. Platform is imx28.
[ 79.66] =
[ 79.66] [ INFO: possible recursive locking detected ]
[ 79.66] 3.4.0-9-g3e96082-dirty #11 Not tainted
[ 79.66] -
[ 79.66] swapper/0 is trying to acquire lock:
[ 79.66] ((host-lock)-rlock#2){-.}, at: [c026ea3c]
mxs_mmc_enable_sdio_irq+0x18/0xd4
[ 79.66]
[ 79.66] but task is already holding lock:
[ 79.66] ((host-lock)-rlock#2){-.}, at: [c026f744]
mxs_mmc_irq_handler+0x1c/0xe8
[ 79.66]
[ 79.66] other info that might help us debug this:
[ 79.66] Possible unsafe locking scenario:
[ 79.66]
[ 79.66]CPU0
[ 79.66]
[ 79.66] lock((host-lock)-rlock#2);
[ 79.66] lock((host-lock)-rlock#2);
[ 79.66]
[ 79.66] *** DEADLOCK ***
[ 79.66]
[ 79.66] May be due to missing lock nesting notation
[ 79.66]
[ 79.66] 1 lock held by swapper/0:
[ 79.66] #0: ((host-lock)-rlock#2){-.}, at: [c026f744]
mxs_mmc_irq_handler+0x1c/0xe8
[ 79.66]
[ 79.66] stack backtrace:
[ 79.66] [c0014bd0] (unwind_backtrace+0x0/0xf4) from
[c005f9c0] (__lock_acquire+0x1948/0x1d48)
[ 79.66] [c005f9c0] (__lock_acquire+0x1948/0x1d48) from
[c005fea0] (lock_acquire+0xe0/0xf8)
[ 79.66] [c005fea0] (lock_acquire+0xe0/0xf8) from [c03a8460]
(_raw_spin_lock_irqsave+0x44/0x58)
[ 79.66] [c03a8460] (_raw_spin_lock_irqsave+0x44/0x58) from
[c026ea3c] (mxs_mmc_enable_sdio_irq+0x18/0xd4)
[ 79.66] [c026ea3c] (mxs_mmc_enable_sdio_irq+0x18/0xd4) from
[c026f7fc] (mxs_mmc_irq_handler+0xd4/0xe8)
[ 79.66] [c026f7fc] (mxs_mmc_irq_handler+0xd4/0xe8) from
[c006bdd8] (handle_irq_event_percpu+0x70/0x254)
[ 79.66] [c006bdd8] (handle_irq_event_percpu+0x70/0x254) from
[c006bff8] (handle_irq_event+0x3c/0x5c)
[ 79.66] [c006bff8] (handle_irq_event+0x3c/0x5c) from
[c006e6d0] (handle_level_irq+0x90/0x110)
[ 79.66] [c006e6d0] (handle_level_irq+0x90/0x110) from
[c006b930] (generic_handle_irq+0x38/0x50)
[ 79.66] [c006b930] (generic_handle_irq+0x38/0x50) from
[c00102fc] (handle_IRQ+0x30/0x84)
[ 79.66] [c00102fc] (handle_IRQ+0x30/0x84) from [c000f058]
(__irq_svc+0x38/0x60)
[ 79.66] [c000f058] (__irq_svc+0x38/0x60) from [c0010520]
(default_idle+0x2c/0x40)
[ 79.66] [c0010520] (default_idle+0x2c/0x40) from [c0010a90]
(cpu_idle+0x64/0xcc)
[ 79.66] [c0010a90] (cpu_idle+0x64/0xcc) from [c04ff858]
(start_kernel+0x244/0x2c8)
[ 79.66] BUG: spinlock lockup on CPU#0, swapper/0
[ 79.66] lock: c398cb2c, .magic: dead4ead, .owner: swapper/0,
.owner_cpu: 0
[ 79.66] [c0014bd0] (unwind_backtrace+0x0/0xf4) from
[c01ddb1c] (do_raw_spin_lock+0xf0/0x144)
[ 79.66] [c01ddb1c] (do_raw_spin_lock+0xf0/0x144) from
[c03a8468] (_raw_spin_lock_irqsave+0x4c/0x58)
[ 79.66] [c03a8468] (_raw_spin_lock_irqsave+0x4c/0x58) from
[c026ea3c] (mxs_mmc_enable_sdio_irq+0x18/0xd4)
[ 79.66] [c026ea3c] (mxs_mmc_enable_sdio_irq+0x18/0xd4) from
[c026f7fc] (mxs_mmc_irq_handler+0xd4/0xe8)
[ 79.66] [c026f7fc] (mxs_mmc_irq_handler+0xd4/0xe8) from
[c006bdd8] (handle_irq_event_percpu+0x70/0x254)
[ 79.66] [c006bdd8] (handle_irq_event_percpu+0x70/0x254) from
[c006bff8] (handle_irq_event+0x3c/0x5c)
[ 79.66] [c006bff8] (handle_irq_event+0x3c/0x5c) from
[c006e6d0] (handle_level_irq+0x90/0x110)
[ 79.66] [c006e6d0] (handle_level_irq+0x90/0x110) from
[c006b930] (generic_handle_irq+0x38/0x50)
[ 79.66] [c006b930] (generic_handle_irq+0x38/0x50) from
[c00102fc] (handle_IRQ+0x30/0x84)
[ 79.66] [c00102fc] (handle_IRQ+0x30/0x84) from [c000f058]
(__irq_svc+0x38/0x60)
[ 79.66] [c000f058] (__irq_svc+0x38/0x60) from [c0010520]
(default_idle+0x2c/0x40)
[ 79.66] [c0010520] (default_idle+0x2c/0x40) from [c0010a90]
(cpu_idle+0x64/0xcc)
[ 79.66] [c0010a90] (cpu_idle+0x64/0xcc) from [c04ff858]
(start_kernel+0x244/0x2c8)
I found a way to fix this issue:
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -278,11 +278,11 @@ static irqreturn_t mxs_mmc_irq_handler(int irq,
void *dev_id)
writel(stat MXS_MMC_IRQ_BITS,
host-base + HW_SSP_CTRL1(host) + STMP_OFFSET_REG_CLR);
+ spin_unlock(host-lock);
+
if ((stat BM_SSP_CTRL1_SDIO_IRQ) (stat BM_SSP_CTRL1_SDIO_IRQ_EN))
mmc_signal_sdio_irq(host-mmc);
- spin_unlock(host-lock);
-
if (stat BM_SSP_CTRL1_RESP_TIMEOUT_IRQ)
cmd-error = -ETIMEDOUT;
else if (stat BM_SSP_CTRL1_RESP_ERR_IRQ)
Is there any reason to keep mmc_signal_sdio_irq inside the spinlock
Re: [PATCH] mmc: omap_hsmmc: fix runtime suspend crash: add host checks in callbacks
S, Venkatraman svenk...@ti.com writes: On Sat, Jul 7, 2012 at 5:56 AM, Kevin Hilman khil...@ti.com wrote: Due to the way the driver core takes runtime PM references during probe, a driver's runtime PM callbacks may not be called until probe returns. During probe, drvdata is set to the 'host' pointer but if probe fails, drvdata is set to NULL. The runtime PM callbacks currently blindly dereference this host pointer, but if probe fails, and the runtime PM callbacks are called after probe returns, a NULL pointer dereference will result. Pardon my ignorance, but why would runtime suspend be called for a device whose probe has failed ? AFAIK, MMC stack wouldn't make the _enable()/disable() calls, which call runtime PM APIs, unless the probe is successful. Is there a stacktrace for the offending caller ? First thing to note: there are several error conditions *after* pm_runtime_enable() and the first _get_sync() are called. So here's what can happen: The driver core does a _get_noresume() before calling the driver's probe, so the runtime PM usecount is already 1 when the driver is called. The _get_sync() in _probe enables the device and increases the usecount to 2. The _put_sync() at the end of -probe() will decrease the usecount to 1, but since the usecount is still non-zero, the driver's callbacks are still not called. It's not until the driver core does its _put_sync (to balance the _get_noresume() that the usecount goes to zero and the driver's callbacks are called. When probe succeeds, this all works fine. However, if probe fails the host pointer is set to NULL, but the runtime PM callbacks are still called and attempt to dereference a NULL pointer. Fix this by simply checking to be sure the host pointer is non-NULL. Signed-off-by: Kevin Hilman khil...@ti.com --- Applies to v3.5-rc5. Fix is needed for v3.5-rc. Can you please let me know which commit introduced this problem in the first place ? There were not many changes in MMC PM code in this merge window. I guess this problem is probably not a regression and has been around for some time. It has not been seen because probe has not failed. In using recent l-o master though, with Russell's dmaengine changes merged, probe is failing for me becasue it can't allocate a DMA channel, and then I'm seeing this problem. Kevin -- 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: omap_hsmmc: fix runtime suspend crash: add host checks in callbacks
On Tue, Jul 10, 2012 at 7:47 PM, Kevin Hilman khil...@ti.com wrote: S, Venkatraman svenk...@ti.com writes: On Sat, Jul 7, 2012 at 5:56 AM, Kevin Hilman khil...@ti.com wrote: Due to the way the driver core takes runtime PM references during probe, a driver's runtime PM callbacks may not be called until probe returns. During probe, drvdata is set to the 'host' pointer but if probe fails, drvdata is set to NULL. The runtime PM callbacks currently blindly dereference this host pointer, but if probe fails, and the runtime PM callbacks are called after probe returns, a NULL pointer dereference will result. Pardon my ignorance, but why would runtime suspend be called for a device whose probe has failed ? AFAIK, MMC stack wouldn't make the _enable()/disable() calls, which call runtime PM APIs, unless the probe is successful. Is there a stacktrace for the offending caller ? First thing to note: there are several error conditions *after* pm_runtime_enable() and the first _get_sync() are called. So here's what can happen: The driver core does a _get_noresume() before calling the driver's probe, so the runtime PM usecount is already 1 when the driver is called. The _get_sync() in _probe enables the device and increases the usecount to 2. The _put_sync() at the end of -probe() will decrease the usecount to 1, but since the usecount is still non-zero, the driver's callbacks are still not called. It's not until the driver core does its _put_sync (to balance the _get_noresume() that the usecount goes to zero and the driver's callbacks are called. Thanks for the detailed explanation. But pm_runtime_disable() is called in the error path, so shouldn't it prevent the driver callbacks from being called ? (The docuementation talks about disable_depth field, but it doesn't sound right that runtime pm would be enabled before _probe() is called. So disable_depth should be 1 when pm_runtime_disable is called in _probe error path, right ?) When probe succeeds, this all works fine. However, if probe fails the host pointer is set to NULL, but the runtime PM callbacks are still called and attempt to dereference a NULL pointer. Fix this by simply checking to be sure the host pointer is non-NULL. Signed-off-by: Kevin Hilman khil...@ti.com --- Applies to v3.5-rc5. Fix is needed for v3.5-rc. Can you please let me know which commit introduced this problem in the first place ? There were not many changes in MMC PM code in this merge window. I guess this problem is probably not a regression and has been around for some time. It has not been seen because probe has not failed. In using recent l-o master though, with Russell's dmaengine changes merged, probe is failing for me becasue it can't allocate a DMA channel, and then I'm seeing this problem. Kevin Ok. I'll let Chris decide on this - DMA engine is not yet in Linus's tree though. -- 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: mmc: mxs: DEADLOCK
Dear Lauri Hintsala, [...] --- a/drivers/mmc/host/mxs-mmc.c +++ b/drivers/mmc/host/mxs-mmc.c @@ -278,11 +278,11 @@ static irqreturn_t mxs_mmc_irq_handler(int irq, void *dev_id) writel(stat MXS_MMC_IRQ_BITS, host-base + HW_SSP_CTRL1(host) + STMP_OFFSET_REG_CLR); + spin_unlock(host-lock); + if ((stat BM_SSP_CTRL1_SDIO_IRQ) (stat BM_SSP_CTRL1_SDIO_IRQ_EN)) mmc_signal_sdio_irq(host-mmc); - spin_unlock(host-lock); - Spinlock in irq handler is interesting too ;-) if (stat BM_SSP_CTRL1_RESP_TIMEOUT_IRQ) cmd-error = -ETIMEDOUT; else if (stat BM_SSP_CTRL1_RESP_ERR_IRQ) Is there any reason to keep mmc_signal_sdio_irq inside the spinlock? mmc_signal_sdio_irq calls mxs_mmc_enable_sdio_irq and it tries to acquire lock while it is already acquired. Best regards, Lauri Hintsala Best regards, Marek Vasut -- 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: Prevent eMMC VCC supply to be cut from late init
On 9 May 2012 19:45, Ulf Hansson ulf.hans...@stericsson.com wrote:
From: Ulf Hansson ulf.hans...@linaro.org
For eMMC cards that has been initialized from a bootloader,
the VCC voltage supply must not be cut in an uncontrolled
manner, without first sending SLEEP or POWEROFF_NOTIFY.
The regulator_init_complete late initcall, may cut the VCC
regulator if it's reference counter is zero. To be able to
prevent the regulator from being cut, mmc_start_host, which
should execute at device init and thus before late init,
calls mmc_power_up. Then the host driver is able to increase
the reference to the regulator.
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
drivers/mmc/core/core.c | 18 +++---
1 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index ba821fe..0b6141d 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -42,6 +42,7 @@
#include sdio_ops.h
static struct workqueue_struct *workqueue;
+static const unsigned freqs[] = { 40, 30, 20, 10 };
/*
* Enabling software CRCs on the data blocks can be a significant (30%)
@@ -1157,6 +1158,9 @@ static void mmc_power_up(struct mmc_host *host)
{
int bit;
+ if (host-ios.power_mode == MMC_POWER_ON)
+ return;
+
mmc_host_clk_hold(host);
/* If ocr is set, we use it */
@@ -1199,6 +1203,10 @@ static void mmc_power_up(struct mmc_host *host)
void mmc_power_off(struct mmc_host *host)
{
int err = 0;
+
+ if (host-ios.power_mode == MMC_POWER_OFF)
+ return;
+
mmc_host_clk_hold(host);
host-ios.clock = 0;
@@ -2005,7 +2013,6 @@ EXPORT_SYMBOL(mmc_detect_card_removed);
void mmc_rescan(struct work_struct *work)
{
- static const unsigned freqs[] = { 40, 30, 20, 10 };
struct mmc_host *host =
container_of(work, struct mmc_host, detect.work);
int i;
@@ -2044,8 +2051,12 @@ void mmc_rescan(struct work_struct *work)
*/
mmc_bus_put(host);
- if (host-ops-get_cd host-ops-get_cd(host) == 0)
+ if (host-ops-get_cd host-ops-get_cd(host) == 0) {
+ mmc_claim_host(host);
+ mmc_power_off(host);
+ mmc_release_host(host);
goto out;
+ }
mmc_claim_host(host);
for (i = 0; i ARRAY_SIZE(freqs); i++) {
@@ -2063,7 +2074,8 @@ void mmc_rescan(struct work_struct *work)
void mmc_start_host(struct mmc_host *host)
{
- mmc_power_off(host);
+ host-f_init = max(freqs[0], host-f_min);
+ mmc_power_up(host);
mmc_detect_change(host, 0);
}
sorry to comment late on this.
This patch breaks the card detect with the designware host controller.
Anybody using the same host controller please let me know.
The problem is caused by the power_up call from the mmc_start_host
function. If i comment this powerup then card detect is fine
--
1.7.9
--
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] mmc: core: Prevent eMMC VCC supply to be cut from late init
Hi,
On Tue, Jul 10 2012, Girish K S wrote:
@@ -2063,7 +2074,8 @@ void mmc_rescan(struct work_struct *work)
void mmc_start_host(struct mmc_host *host)
{
- mmc_power_off(host);
+ host-f_init = max(freqs[0], host-f_min);
+ mmc_power_up(host);
mmc_detect_change(host, 0);
}
sorry to comment late on this.
This patch breaks the card detect with the designware host controller.
Anybody using the same host controller please let me know.
The problem is caused by the power_up call from the mmc_start_host
function. If i comment this powerup then card detect is fine
Thanks. Ulf, sounds like we should revert this for 3.5-rc (which would
have to happen soon) while we investigate; let me know what you think.
- 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
Re: [CFT 03/11] mmc: omap_hsmmc: remove private DMA API implementation
Hi Russell,
Russell King rmk+ker...@arm.linux.org.uk writes:
Remove the private DMA API implementation from omap_hsmmc, making it
use entirely the DMA engine API.
Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
While testing this, I noticed a minor problem in the case of probe
failure (e.g. if dmaengine is not built into the kernel.)
The current driver suffers from this same problem but should probably be
fixed when converting to dmaengine...
[...]
@@ -2048,36 +1919,28 @@ static int __devinit omap_hsmmc_probe(struct
platform_device *pdev)
dev_err(mmc_dev(host-mmc), cannot get DMA TX channel\n);
goto err_irq;
}
- host-dma_line_tx = res-start;
+ tx_req = res-start;
res = platform_get_resource_byname(pdev, IORESOURCE_DMA, rx);
if (!res) {
dev_err(mmc_dev(host-mmc), cannot get DMA RX channel\n);
goto err_irq;
}
- host-dma_line_rx = res-start;
+ rx_req = res-start;
- {
- dma_cap_mask_t mask;
- unsigned sig;
- extern bool omap_dma_filter_fn(struct dma_chan *chan, void
*param);
-
- dma_cap_zero(mask);
- dma_cap_set(DMA_SLAVE, mask);
-#if 1
- sig = host-dma_line_rx;
- host-rx_chan = dma_request_channel(mask, omap_dma_filter_fn,
sig);
- if (!host-rx_chan) {
- dev_warn(mmc_dev(host-mmc), unable to obtain RX DMA
engine channel %u\n, sig);
- }
-#endif
-#if 1
- sig = host-dma_line_tx;
- host-tx_chan = dma_request_channel(mask, omap_dma_filter_fn,
sig);
- if (!host-tx_chan) {
- dev_warn(mmc_dev(host-mmc), unable to obtain TX DMA
engine channel %u\n, sig);
- }
-#endif
+ dma_cap_zero(mask);
+ dma_cap_set(DMA_SLAVE, mask);
+
+ host-rx_chan = dma_request_channel(mask, omap_dma_filter_fn, rx_req);
+ if (!host-rx_chan) {
+ dev_err(mmc_dev(host-mmc), unable to obtain RX DMA engine
channel %u\n, rx_req);
+ goto err_irq;
+ }
+
+ host-tx_chan = dma_request_channel(mask, omap_dma_filter_fn, tx_req);
+ if (!host-tx_chan) {
+ dev_err(mmc_dev(host-mmc), unable to obtain TX DMA engine
channel %u\n, tx_req);
+ goto err_irq;
}
If either of these fails, ret is zero so even though this results in a
failed probe, the return value (ret) is zero meaning the driver still
gets bound to the device.
The patch below fixes this and applies on your 'for-next' branch. Or,
feel free to fold this into the original if you prefer.
Kevin
From af7537997b46ee3991985fecd4b4a302bdc0df31 Mon Sep 17 00:00:00 2001
From: Kevin Hilman khil...@ti.com
Date: Tue, 10 Jul 2012 14:30:18 -0700
Subject: [PATCH] mmc: omap_hsmmc: ensure probe returns error if DMA channel
request fails
If dma_request_channel() fails (e.g. because DMA engine is not built
into the kernel), the return value from probe is zero causing the
driver to be bound to the device even though probe failed.
To fix, ensure that probe returns an error value when a DMA channel
request fail.
Cc: Russell King rmk+ker...@arm.linux.org.uk
Signed-off-by: Kevin Hilman khil...@ti.com
---
drivers/mmc/host/omap_hsmmc.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 2338703..ddcecf8 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1924,12 +1924,14 @@ static int __devinit omap_hsmmc_probe(struct
platform_device *pdev)
host-rx_chan = dma_request_channel(mask, omap_dma_filter_fn, rx_req);
if (!host-rx_chan) {
dev_err(mmc_dev(host-mmc), unable to obtain RX DMA engine
channel %u\n, rx_req);
+ ret = -ENXIO;
goto err_irq;
}
host-tx_chan = dma_request_channel(mask, omap_dma_filter_fn, tx_req);
if (!host-tx_chan) {
dev_err(mmc_dev(host-mmc), unable to obtain TX DMA engine
channel %u\n, tx_req);
+ ret -ENXIO;
goto err_irq;
}
--
1.7.9.2
--
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: omap_hsmmc: fix runtime suspend crash: add host checks in callbacks
S, Venkatraman svenk...@ti.com writes: On Tue, Jul 10, 2012 at 7:47 PM, Kevin Hilman khil...@ti.com wrote: S, Venkatraman svenk...@ti.com writes: On Sat, Jul 7, 2012 at 5:56 AM, Kevin Hilman khil...@ti.com wrote: Due to the way the driver core takes runtime PM references during probe, a driver's runtime PM callbacks may not be called until probe returns. During probe, drvdata is set to the 'host' pointer but if probe fails, drvdata is set to NULL. The runtime PM callbacks currently blindly dereference this host pointer, but if probe fails, and the runtime PM callbacks are called after probe returns, a NULL pointer dereference will result. Pardon my ignorance, but why would runtime suspend be called for a device whose probe has failed ? AFAIK, MMC stack wouldn't make the _enable()/disable() calls, which call runtime PM APIs, unless the probe is successful. Is there a stacktrace for the offending caller ? First thing to note: there are several error conditions *after* pm_runtime_enable() and the first _get_sync() are called. So here's what can happen: The driver core does a _get_noresume() before calling the driver's probe, so the runtime PM usecount is already 1 when the driver is called. The _get_sync() in _probe enables the device and increases the usecount to 2. The _put_sync() at the end of -probe() will decrease the usecount to 1, but since the usecount is still non-zero, the driver's callbacks are still not called. It's not until the driver core does its _put_sync (to balance the _get_noresume() that the usecount goes to zero and the driver's callbacks are called. Thanks for the detailed explanation. But pm_runtime_disable() is called in the error path, so shouldn't it prevent the driver callbacks from being called ? Yes, you're right. I'm getting a few different problems mixed together here. To work on a different problem, I had commented out the pm_runtime_disable() call in the error path and so I was seeing the callbacks being called even on probe failure. Withe the pm_runtime_disable() added back, that doesn't happen. However, there is another case where the runtime PM callbacks may be called: the PM domain code in omap_device can still attempts to call the runtime PM callbacks during late suspend for devices that are not already runtime suspended. This is done even when there is no driver bound to a device, but this is a bug in the omap_device code, not in the MMC driver. So I'll be sending a patch shortly to fix that. So, all that to say, this patch can be dropped. Thanks for the detailed review. Kevin -- 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: omap_hsmmc: ensure probe returns error upon resource failure
If platform_get_resource_by_name() fails, driver probe is aborted an
should return an error so the driver is not bound to the device.
However, in the current error path of platform_get_resource_by_name(),
probe returns zero since the return value (ret) is not properly set.
With a zero return value, the driver core assumes probe was successful
and will bind the driver to the device.
Fix this by ensuring that probe returns an error code in this failure
path.
Signed-off-by: Kevin Hilman khil...@ti.com
---
drivers/mmc/host/omap_hsmmc.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 389a3ee..19e60e9 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1931,6 +1931,7 @@ static int __devinit omap_hsmmc_probe(struct
platform_device *pdev)
res = platform_get_resource_byname(pdev, IORESOURCE_DMA, tx);
if (!res) {
dev_err(mmc_dev(host-mmc), cannot get DMA TX channel\n);
+ ret = -ENXIO;
goto err_irq;
}
host-dma_line_tx = res-start;
@@ -1938,6 +1939,7 @@ static int __devinit omap_hsmmc_probe(struct
platform_device *pdev)
res = platform_get_resource_byname(pdev, IORESOURCE_DMA, rx);
if (!res) {
dev_err(mmc_dev(host-mmc), cannot get DMA RX channel\n);
+ ret = -ENXIO;
goto err_irq;
}
host-dma_line_rx = res-start;
--
1.7.9.2
--
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 5/5] mmc: card: Add RPMB support in IOCTL interface
2012/7/10, Loic pallardy loic.palla...@gmail.com:
Repost in text format
2012/7/10 Namjae Jeon linkinj...@gmail.com
+ err = mmc_blk_part_switch(card, md);
+ if (err)
+ goto cmd_rel_host;
+
Should it wrapped by if (is_rpbm) condition ? It can be called in other
ioctl.
It was my first implementation, but in that case you have to manage a
session, switching on RPMB partition at the begining of the IOCTL and
restore previous partition at the end. (because RPMB partition doesn't
support all commands).
Having a look to how mmc driver manages partition selection, I saw
that mmc_blk_issue_rq for instance calls mmc_blk_part_switch in the
beginning to be sure it's on the right partition, but never restores
The proposal is to use same way and to make it compliant with Boot and
GP partitions if there are any special commands supported by them.
Okay, I understood what you mean.
we should change partition switching in other ioctl again after ioctl
about rpmb is called.
Reviewed-by: Namjae Jeon linkinj...@gmail.com
if (idata-ic.is_acmd) {
err = mmc_app_cmd(card-host, card);
if (err)
goto cmd_rel_host;
}
--
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 v1 2/5] mmc: card: Do not scan RPMB partitions
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c index 276d21c..1b202fe 100644 --- a/drivers/mmc/card/block.c +++ b/drivers/mmc/card/block.c @@ -1521,6 +1521,9 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card, md-disk-queue = md-queue.queue; md-disk-driverfs_dev = parent; set_disk_ro(md-disk, md-read_only || default_ro); + md-disk-flags = GENHD_FL_EXT_DEVT; Currrenlty mmc have been setting minors to CONFIG_MMC_BLOCK_MINORS. Why did you add GENHD_FL_EXT_DEVT ? 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
