[PATCH] mmc: block: fixed NULL pointer dereference
Hi.
I send to mailing for [RFC] Kernel NULL pointer dereference.
This patch is fixed it.
In similar case, when discard request, check condition and
performed mmc_blk_issue_rw_rq(mq, NULL) for ongoing async transfer.
But When flush request, entered the mmc_blk_issue_flush() then return.
(then didn't complete ongoing aync transfer).
I think that need to complete for ongoing aync transfer before flush request.
I tested with this patch, it's working fine.
(SDHCI controller, eMMC4.41)
Signed-off-by: Jaehoon Chung jh80.ch...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/mmc/card/block.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 38d0149..1ff5486 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1200,6 +1200,9 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, struct
request *req)
else
ret = mmc_blk_issue_discard_rq(mq, req);
} else if (req req-cmd_flags REQ_FLUSH) {
+ /* complete ongoing async transfer before issuing flush */
+ if (card-host-areq)
+ mmc_blk_issue_rw_rq(mq, NULL);
ret = mmc_blk_issue_flush(mq, req);
} else {
ret = mmc_blk_issue_rw_rq(mq, req);
--
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: [PATCHv4 2/3] MMC: OMAP: HSMMC: add runtime pm support
-Original Message-
From: linux-mmc-ow...@vger.kernel.org
[mailto:linux-mmc-ow...@vger.kernel.org] On Behalf Of Kevin Hilman
Sent: Saturday, July 09, 2011 2:24 AM
To: Balaji T K
Cc: linux-o...@vger.kernel.org; linux-mmc@vger.kernel.org; c...@laptop.org;
t...@atomide.com; madhu...@ti.com; b-cous...@ti.com; p...@pwsan.com;
kishore.kadiy...@ti.com
Subject: Re: [PATCHv4 2/3] MMC: OMAP: HSMMC: add runtime pm support
Balaji T K balaj...@ti.com writes:
add runtime pm support to HSMMC host controller
Use runtime pm API to enable/disable HSMMC clock
Use runtime autosuspend APIs to enable auto suspend delay
Based on OMAP HSMMC runtime implementation by Kevin Hilman, Kishore
Kadiyala
Signed-off-by: Balaji T K balaj...@ti.com
It's not relevant for this merge window, but I'm exploring some future
changes to our PM core code and have a question about how MMC works for
system suspend.
Basially, the question is: can the driver be reworked such that a system
suspend does not need to runtime resume the device? For most devices,
we kind of expect that if the device is runtime suspended, a system
suspend will have nothing extra to do, but this driver runtime resumes
the device during system suspend in order to do stuff, which I
admitedly don't fully undestand.
Ideally, the stuff needed for runtime suspend and system suspend could
be made to be common such that a system suspend of a runtime suspended
device would be a noop.
Is this possible?
Kevin
During system suspended patch, a callback named .prepare will be first done
before .suspend is called, and .complete callback will be called after .resume
is called. These two callbacks are in pair. If driver can implement the
.prepare and hold the usage count in this callback, then runtime pm
suspend/resume will not happen during device suspending. So there will be no
need to add pm_runtime_get* and pm_runtime_put* in .suspend/.resume.
BTW, if .prepare has hold the usage count, then .complete callback need to
release the usage count and put device in runtime suspended mode.
Thanks
Chuanxiao
@@ -2100,6 +2087,7 @@ static int omap_hsmmc_suspend(struct device *dev)
return 0;
if (host) {
+ pm_runtime_get_sync(host-dev);
host-suspended = 1;
if (host-pdata-suspend) {
ret = host-pdata-suspend(pdev-dev,
@@ -2114,13 +2102,11 @@ static int omap_hsmmc_suspend(struct device
*dev)
}
cancel_work_sync(host-mmc_carddetect_work);
ret = mmc_suspend_host(host-mmc);
- mmc_host_enable(host-mmc);
+
if (ret == 0) {
omap_hsmmc_disable_irq(host);
OMAP_HSMMC_WRITE(host-base, HCTL,
OMAP_HSMMC_READ(host-base, HCTL) ~SDBP);
- mmc_host_disable(host-mmc);
- clk_disable(host-iclk);
if (host-got_dbclk)
clk_disable(host-dbclk);
} else {
@@ -2132,9 +2118,8 @@ static int omap_hsmmc_suspend(struct device *dev)
dev_dbg(mmc_dev(host-mmc),
Unmask interrupt failed\n);
}
- mmc_host_disable(host-mmc);
}
-
+ pm_runtime_put_sync(host-dev);
}
return ret;
}
--
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: block: fixed NULL pointer dereference
On 13 July 2011 10:02, Jaehoon Chung jh80.ch...@samsung.com wrote:
Hi.
I send to mailing for [RFC] Kernel NULL pointer dereference.
This patch is fixed it.
In similar case, when discard request, check condition and
performed mmc_blk_issue_rw_rq(mq, NULL) for ongoing async transfer.
But When flush request, entered the mmc_blk_issue_flush() then return.
(then didn't complete ongoing aync transfer).
I think that need to complete for ongoing aync transfer before flush request.
I did consider this when I added the check for discard but I didn't
realize it is necessary for flush too.
There is no real downside of completing the async request before flush
in terms of performance anyway.
Thanks for catching this bug.
@@ -1200,6 +1200,9 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq,
struct request *req)
else
ret = mmc_blk_issue_discard_rq(mq, req);
} else if (req req-cmd_flags REQ_FLUSH) {
+ /* complete ongoing async transfer before issuing flush */
+ if (card-host-areq)
+ mmc_blk_issue_rw_rq(mq, NULL);
These 2 lines are the same as for the discard condition. I still
prefer 2 lines of repeating code rather than one big if-statement
covering both discard and flush.
Acked-by: Per Forlin per.for...@linaro.org
Regards,
Per
--
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 00/16] omap_hsmmc patches
Hello,
it seems this series got lost in time, at least patches 01, 02, 03,
04, 05, 06, 07, 08, 12 look like valid standalone fixes and
improvements to me, they don't touch other trees and still apply
cleanly, so would be good to have.
Chris, can you queue these for upcoming merge window?
These patches are also archived in patchwork here:
https://patchwork.kernel.org/project/linux-omap/list/?archive=bothpage=8
On Fri, May 6, 2011 at 12:13 PM, Adrian Hunter adrian.hun...@nokia.com wrote:
Hi
Here is V2 of some patches for omap_hsmmc.
Changes in V2:
OMAP: sDMA: descriptor autoloading feature
- removed the feature entirely as per Tony Lindgren
mmc: omap_hsmmc: fix few bugs when set the clock divisor
- added cpu_relax() as per Grazvydas Ignotas
mmc: omap_hsmmc: fix missing mmc_release_host() in no_off case
- removed {} as per Sergei Shtylyov
OMAP: hsmmc: implement clock switcher
- changed to = threshold as per Grazvydas Ignotas
Adrian Hunter (6):
mmc: omap_hsmmc: correct debug report error status mnemonics
OMAP: board-rm680: set MMC nomux flag
mmc: omap_hsmmc: ensure pbias configuration is always done
mmc: omap_hsmmc: fix oops in omap_hsmmc_dma_cb
OMAP: hsmmc: add platform data for eMMC hardware reset gpio
mmc: omap_hsmmc: add a hardware reset before initialization
Andy Shevchenko (8):
mmc: omap_hsmmc: move hardcoded frequency constants to definition block
mmc: omap_hsmmc: reduce a bit the error handlers in probe()
mmc: omap_hsmmc: split duplicate code to calc_divisor() function
mmc: omap_hsmmc: introduce start_clock and re-use stop_clock
mmc: omap_hsmmc: fix few bugs when set the clock divisor
mmc: omap_hsmmc: split same pieces of code to separate functions
OMAP: hsmmc: implement clock switcher
mmc: omap_hsmmc: adjust host controller clock in regard to current OPP
Jarkko Lavinen (1):
OMAP: hsmmc: Do not mux the slot if non default muxing is already done
Sudhir Bera (1):
mmc: omap_hsmmc: fix missing mmc_release_host() in no_off case
arch/arm/mach-omap2/board-rm680.c | 1 +
arch/arm/mach-omap2/hsmmc.c | 188 +-
arch/arm/mach-omap2/hsmmc.h | 2 +
arch/arm/plat-omap/include/plat/mmc.h | 9 +
drivers/mmc/host/omap_hsmmc.c | 353
-
5 files changed, 414 insertions(+), 139 deletions(-)
Regards
Adrian
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Gražvydas
--
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 v6] mmc: documentation of mmc non-blocking request usage and design.
Hi Per, On Sun, Jul 10 2011, Per Forlin wrote: Documentation about the background and the design of mmc non-blocking. Host driver guidelines to minimize request preparation overhead. Signed-off-by: Per Forlin per.for...@linaro.org Acked-by: Randy Dunlap rdun...@xenotime.net Pushed v6 to mmc-next for 3.1, 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
[RFC/RFT] MMC: CORE: eMMC in Sleep mode before suspend
Put MMC to sleep if it supports SLEEP/AWAKE (CMD5)
in the mmc suspend to minimize power consumption.
Signed-off-by: Balaji T K balaj...@ti.com
---
drivers/mmc/core/mmc.c |8 ++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index afabdc3..e8dfcde 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -900,16 +900,20 @@ static void mmc_detect(struct mmc_host *host)
*/
static int mmc_suspend(struct mmc_host *host)
{
+ int err = 0;
+
BUG_ON(!host);
BUG_ON(!host-card);
mmc_claim_host(host);
- if (!mmc_host_is_spi(host))
+ if (mmc_card_can_sleep(host))
+ err = mmc_card_sleep(host);
+ else if (!mmc_host_is_spi(host))
mmc_deselect_cards(host);
host-card-state = ~MMC_STATE_HIGHSPEED;
mmc_release_host(host);
- return 0;
+ return err;
}
/*
--
1.7.0.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] mmc: Added quirks for Ricoh 1180:e823 lower base clock frequency
Hi Arnd, On Tue, Jul 12 2011, Arnd Bergmann wrote: I would very much expect that to be nonreproducible. The first row in each test is the result of a single write() system call and does not get averaged out. More importantly the time for each write depends a lot of the state of the card before the write. For instance when you do a lot of random writes to a card, optionally take it out and put it into a different machine, and then do a large linear write, that linear write will be very slow because the card has to garbage collect all the random writes that were done earlier. After a few writes (usually one is enough), it gets back to the full performance. That makes sense. Do you think this explains Manoj getting a slower first file copy speed (757ms vs. 480ms) after applying his patch? (Manoj, perhaps you could retry your test without GC being needed?) What would we expect lowering the SD base clock frequency from 200MHz to 50MHz to do to performance theoretically? 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
Re: [PATCHv4 2/3] MMC: OMAP: HSMMC: add runtime pm support
Dong, Chuanxiao chuanxiao.d...@intel.com writes: [...] Basially, the question is: can the driver be reworked such that a system suspend does not need to runtime resume the device? For most devices, we kind of expect that if the device is runtime suspended, a system suspend will have nothing extra to do, but this driver runtime resumes the device during system suspend in order to do stuff, which I admitedly don't fully undestand. Ideally, the stuff needed for runtime suspend and system suspend could be made to be common such that a system suspend of a runtime suspended device would be a noop. Is this possible? Kevin During system suspended patch, a callback named .prepare will be first done before .suspend is called, and .complete callback will be called after .resume is called. These two callbacks are in pair. If driver can implement the .prepare and hold the usage count in this callback, then runtime pm suspend/resume will not happen during device suspending. So there will be no need to add pm_runtime_get* and pm_runtime_put* in .suspend/.resume. That doesn't avoid the problem, since the device is still runtime resumed and then re-suspended during system suspend. My basic question is this: why does this device need to be runtime resumed during system suspend? Why can't it just stay runtime suspended? 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: [PATCHv4 2/3] MMC: OMAP: HSMMC: add runtime pm support
On Wed, Jul 13, 2011 at 8:29 PM, Kevin Hilman khil...@ti.com wrote: Dong, Chuanxiao chuanxiao.d...@intel.com writes: [...] Basially, the question is: can the driver be reworked such that a system suspend does not need to runtime resume the device? For most devices, we kind of expect that if the device is runtime suspended, a system suspend will have nothing extra to do, but this driver runtime resumes the device during system suspend in order to do stuff, which I admitedly don't fully undestand. Ideally, the stuff needed for runtime suspend and system suspend could be made to be common such that a system suspend of a runtime suspended device would be a noop. Is this possible? Kevin During system suspended patch, a callback named .prepare will be first done before .suspend is called, and .complete callback will be called after .resume is called. These two callbacks are in pair. If driver can implement the .prepare and hold the usage count in this callback, then runtime pm suspend/resume will not happen during device suspending. So there will be no need to add pm_runtime_get* and pm_runtime_put* in .suspend/.resume. That doesn't avoid the problem, since the device is still runtime resumed and then re-suspended during system suspend. My basic question is this: why does this device need to be runtime resumed during system suspend? Why can't it just stay runtime suspended? From my understanding, the runtime suspend is usually implemented to not lose the card 'context', i.e. transactions can continue after a runtime suspend / resume cycle. For system suspend, the MMC core sends a sleep command (which, in itself, is a transaction) to the card to go to sleep state, and for all practical purposes, the card is treated as 'removed'. When the system resumes, the card is rescanned and re-initialized. Hence, for system suspend, the MMC controller needs to be enabled to actually send the command which puts the card to sleep (and hence the resume). Best regards, Venkat. -- 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 00/16] omap_hsmmc patches
Hi,
On Wed, Jul 13 2011, Grazvydas Ignotas wrote:
it seems this series got lost in time, at least patches 01, 02, 03,
04, 05, 06, 07, 08, 12 look like valid standalone fixes and
improvements to me, they don't touch other trees and still apply
cleanly, so would be good to have.
Yeah. I didn't merge them because Tony still has unanswered comments on
the OMAP side of the patches, and they're submitted as a single patchset.
I've applied {2, 3, 5, 6, 7, 8, 12} to mmc-next for 3.1 now. 1 doesn't
apply because the function doesn't exist anymore. 4 doesn't apply
because iclk doesn't exist anymore.
If someone wants to look at rebasing the rest of these against mmc-next
and resubmitting, I think that'd be useful.
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
Re: [PATCH v3 4/4] mmc: sdhci-esdhc-imx: add device tree probe support
Hi,
On Wed, Jul 06, 2011 at 03:05:18PM -0600, Grant Likely wrote:
On Thu, Jul 07, 2011 at 12:47:50AM +0800, Shawn Guo wrote:
The patch adds device tree probe support for sdhci-esdhc-imx driver.
Signed-off-by: Shawn Guo shawn@linaro.org
Cc: Wolfram Sang w.s...@pengutronix.de
Cc: Chris Ball c...@laptop.org
Cc: Grant Likely grant.lik...@secretlab.ca
Acked-by: Grant Likely grant.lik...@secretlab.ca
[...]
+Optional properties:
+- fsl,card-wired : Indicate the card is wired to host permanently
+- fsl,cd-internal : Indicate to use controller internal card detection
+- fsl,wp-internal : Indicate to use controller internal write protection
+- cd-gpios : Specify GPIOs for card detection
+- wp-gpios : Specify GPIOs for write protection
[...]
+ cd-gpios = gpio0 6 0; /* GPIO1_6 */
+ wp-gpios = gpio0 5 0; /* GPIO1_5 */
Is there any particular reason why we started using named GPIOs
in the device tree? To me, that's quite drastic change... should
we start using named regs and interrupts as well? Personally, I
don't think that the idea is great, as now you don't know where
to expect memory resources, interrupt resources and, eventually
GPIO resources.
Just a few disadvantages off the top of my head:
- You can't statically validate your device tree for correctness.
Today dtc checks #{address,size}-cells sanity for 'regs' and
'ranges';
- You can't automatically convert named resources into 'struct
resource *', as we do for platform devices now;
- Any pros for using named resources in the device tree? I don't
see any.
So, I suggest to at least discuss this stuff a little bit more
before polluting device trees with dubious ideas.
p.s.
As for this particular patch, I really don't see why we should
use named GPIOs. For consistency, I'd suggest to reuse bindings
from Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt.
Plus, fsl,cd-internal and fsl,wp-internal is not needed: either
you specify GPIOs or not. That already signals whether driver
should use internal detection (i.e. 'present' register) or
external (i.e. GPIO).
And also, why {cd,wp}-gpioS (plural)?
--
Anton Vorontsov
Email: cbouatmai...@gmail.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: [PATCHv4 2/3] MMC: OMAP: HSMMC: add runtime pm support
S, Venkatraman svenk...@ti.com writes: On Wed, Jul 13, 2011 at 8:29 PM, Kevin Hilman khil...@ti.com wrote: [...] My basic question is this: why does this device need to be runtime resumed during system suspend? Why can't it just stay runtime suspended? From my understanding, the runtime suspend is usually implemented to not lose the card 'context', i.e. transactions can continue after a runtime suspend / resume cycle. For system suspend, the MMC core sends a sleep command (which, in itself, is a transaction) to the card to go to sleep state, and for all practical purposes, the card is treated as 'removed'. When the system resumes, the card is rescanned and re-initialized. Hence, for system suspend, the MMC controller needs to be enabled to actually send the command which puts the card to sleep (and hence the resume). Great, this is the detail I was looking for since my MMC knowledge is quite limited. So, in theory, this same sleep command could be sent during runtime suspend as well, so that a system suspend would not have to runtime resume, correct? But I suppose it would result in a much higher latency runtime resumes. It might be worth experimenting with doing this, possibly in combination with longer auto-suspend delay times. Thanks for the explanation, 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 v3 4/4] mmc: sdhci-esdhc-imx: add device tree probe support
On Wed, 13 Jul 2011 19:52:12 +0400
Anton Vorontsov cbouatmai...@gmail.com wrote:
On Wed, Jul 06, 2011 at 03:05:18PM -0600, Grant Likely wrote:
On Thu, Jul 07, 2011 at 12:47:50AM +0800, Shawn Guo wrote:
+- cd-gpios : Specify GPIOs for card detection
+- wp-gpios : Specify GPIOs for write protection
[...]
+ cd-gpios = gpio0 6 0; /* GPIO1_6 */
+ wp-gpios = gpio0 5 0; /* GPIO1_5 */
Is there any particular reason why we started using named GPIOs
in the device tree? To me, that's quite drastic change... should
we start using named regs and interrupts as well? Personally, I
don't think that the idea is great, as now you don't know where
to expect memory resources, interrupt resources and, eventually
GPIO resources.
Just check the binding, and you'll know. :-)
It makes it easier to deal with cases where some resources are optional,
especially when dealing with a binding for a family of devices that grows
over time, and helps avoid resources being mismatched since order no longer
matters.
Just a few disadvantages off the top of my head:
- You can't statically validate your device tree for correctness.
Today dtc checks #{address,size}-cells sanity for 'regs' and
'ranges';
The vast majority of stuff in the device tree already cannot be checked in
this manner, without adding binding knowledge to dtc.
Perhaps it could check things that end in -gpios, -regs, etc., if we
avoid adding new properties that match those patterns that aren't what they
appear to be, and let dtc know about any existing cases.
- You can't automatically convert named resources into 'struct
resource *', as we do for platform devices now;
So add named resource support to platform devices. :-)
-Scott
--
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 v3 4/4] mmc: sdhci-esdhc-imx: add device tree probe support
On Wed, Jul 13, 2011 at 11:09:40AM -0500, Scott Wood wrote:
[...]
- You can't automatically convert named resources into 'struct
resource *', as we do for platform devices now;
So add named resource support to platform devices. :-)
There are surely no technical difficulties, it's more about 'type
safety', and as a consequence -- resources being discoverable.
If we want to use named resources, I think we should explicitly reserve
the *-{regs,interrupts,gpios} suffixes for such purposes. And once such
a reservation is done, we're safe again -- resources become discoverable.
Thanks,
--
Anton Vorontsov
Email: cbouatmai...@gmail.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: [PATCH] mmc: Added quirks for Ricoh 1180:e823 lower base clock frequency
On Wednesday 13 July 2011, Manoj Iyer wrote: Chris/Arnd, Here is a series of test I did with the patched kernel. == cold boot insert SD card == u@u:~/flash/flashbench$ sudo ./flashbench -O --erasesize=$[4 * 1024 * 1024] --blocksize=$[256 * 1024] /dev/mmcblk0 --open-au-nr=2 4MiB4.96M/s 2MiB6.3M/s 1MiB6.23M/s 512KiB 6.23M/s 256KiB 6.26M/s The very first one obviously triggers a garbage collection. Everything after that is well within measuring accuracy around 6.25MB/s On Wed, 13 Jul 2011, Chris Ball wrote: That makes sense. Do you think this explains Manoj getting a slower first file copy speed (757ms vs. 480ms) after applying his patch? (Manoj, perhaps you could retry your test without GC being needed?) Yes. For a single sample, it can easily explain differences up to 500ms. You have to average out file system benchmarks across a lot of files to be sure. What would we expect lowering the SD base clock frequency from 200MHz to 50MHz to do to performance theoretically? Not much. This card only has a 6MB/s write speed, which is well below what a 50 MHz bus can do. It mgiht be different on a fast eMMC device or a Sandisk Extreme Pro UHS-1 card. Arnd -- 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 v3 4/4] mmc: sdhci-esdhc-imx: add device tree probe support
On Thu, Jul 14, 2011 at 12:52 AM, Anton Vorontsov
cbouatmai...@gmail.com wrote:
Hi,
On Wed, Jul 06, 2011 at 03:05:18PM -0600, Grant Likely wrote:
On Thu, Jul 07, 2011 at 12:47:50AM +0800, Shawn Guo wrote:
The patch adds device tree probe support for sdhci-esdhc-imx driver.
Signed-off-by: Shawn Guo shawn@linaro.org
Cc: Wolfram Sang w.s...@pengutronix.de
Cc: Chris Ball c...@laptop.org
Cc: Grant Likely grant.lik...@secretlab.ca
Acked-by: Grant Likely grant.lik...@secretlab.ca
[...]
+Optional properties:
+- fsl,card-wired : Indicate the card is wired to host permanently
+- fsl,cd-internal : Indicate to use controller internal card detection
+- fsl,wp-internal : Indicate to use controller internal write protection
+- cd-gpios : Specify GPIOs for card detection
+- wp-gpios : Specify GPIOs for write protection
[...]
+ cd-gpios = gpio0 6 0; /* GPIO1_6 */
+ wp-gpios = gpio0 5 0; /* GPIO1_5 */
Is there any particular reason why we started using named GPIOs
in the device tree? To me, that's quite drastic change... should
we start using named regs and interrupts as well? Personally, I
don't think that the idea is great, as now you don't know where
to expect memory resources, interrupt resources and, eventually
GPIO resources.
No, there are no plans to move to named irqs or regs. irq and reg
resources tend to be a lot more regular than gpios. It's not a
drastic change though because existing bindings remain unaffected, and
new bindings can still use unnamed gpios if that is more appropriate.
Just a few disadvantages off the top of my head:
- You can't statically validate your device tree for correctness.
Today dtc checks #{address,size}-cells sanity for 'regs' and
'ranges';
We can. The gpio binding was extended to be in the form
[name-]gpios. Any property with the string gpios at the end can
be statically validated.
- You can't automatically convert named resources into 'struct
resource *', as we do for platform devices now;
Only for irqs and regs. gpios have never been automatically loaded
into resources.
- Any pros for using named resources in the device tree? I don't
see any.
Human readability. To know exactly what a gpio is intended to be used
for. Particularly for the case where a device might not use all the
gpios that it could use. Yes, the gpios property can have 'holes' in
it, but the observation was made by several people that it is easy to
get wrong. I for one thing the concern was well justified.
So, I suggest to at least discuss this stuff a little bit more
before polluting device trees with dubious ideas.
It was discussed on list quite a while ago.
p.s.
As for this particular patch, I really don't see why we should
use named GPIOs. For consistency, I'd suggest to reuse bindings
from Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt.
Plus, fsl,cd-internal and fsl,wp-internal is not needed: either
you specify GPIOs or not. That already signals whether driver
should use internal detection (i.e. 'present' register) or
external (i.e. GPIO).
wp-internal and cd-internal differentiates between using the internal
functionality and not having wp/cd at all.
And also, why {cd,wp}-gpioS (plural)?
For consistency. Doing it that way means that the plural gpios is
always the suffix for both the gpios and name-gpios use cases.
--
Anton Vorontsov
Email: cbouatmai...@gmail.com
___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
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: Added quirks for Ricoh 1180:e823 lower base clock frequency
On Wed, Jul 13 2011, Arnd Bergmann wrote: Not much. This card only has a 6MB/s write speed, which is well below what a 50 MHz bus can do. It mgiht be different on a fast eMMC device or a Sandisk Extreme Pro UHS-1 card. Okay, I see. I think we're good for now, then -- hopefully anyone using a UHS card on their laptop will see the printk when wondering why their performance isn't great and get in touch with us, and we can investigate more then. I've pushed Manoj's patch to mmc-next for 3.1 now, with the printk changes and a stable@ tag. Thanks Chris! Thanks again, - Chris. -- Chris Ball c...@laptop.org http://printf.net/ One Laptop Per Child -- Manoj Iyer Ubuntu/Canonical Hardware Enablement -- 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: Added quirks for Ricoh 1180:e823 lower base clock frequency
Hi, On Wed, Jul 13 2011, Manoj Iyer wrote: Do you think that lowering the controller speed to 50Mhz in case we have a failure is a better idea than reducing the speed for all e823 ricoh controllers? I can send a V2 of the patch. What do you think ? It could be better. Do you think you could get hold of a UHS-1 card for testing with? (I don't think it's worth doing if there's no appreciable performance difference either way.) 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
Re: [PATCH] mmc: Added quirks for Ricoh 1180:e823 lower base clock frequency
It could be better. Do you think you could get hold of a UHS-1 card for testing with? (I don't think it's worth doing if there's no appreciable performance difference either way.) Yes, I can go out and buy one today, do the performance tests like I have done before and report back to you. Thanks, - Chris. -- Chris Ball c...@laptop.org http://printf.net/ One Laptop Per Child -- Manoj Iyer Ubuntu/Canonical Hardware Enablement -- 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: Added quirks for Ricoh 1180:e823 lower base clock frequency
Hi, On Wed, Jul 13 2011, Manoj Iyer wrote: It could be better. Do you think you could get hold of a UHS-1 card for testing with? (I don't think it's worth doing if there's no appreciable performance difference either way.) Yes, I can go out and buy one today, do the performance tests like I have done before and report back to you. Perfect. Thanks very much! - 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
[GIT PULL] MMC fix for 3.0.
Hi Linus, Please pull from: git://git.kernel.org/pub/scm/linux/kernel/git/cjb/mmc.git for-linus to receive a tested MMC regression fix against 3.0-rc1. Thanks. The following changes since commit 8d86e5f91440aa56a5df516bf58fe3883552ad56: Merge branch 'merge' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2011-07-12 14:21:19 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/cjb/mmc.git for-linus Philip Rakity (1): mmc: core: Bus width testing needs to handle suspend/resume drivers/mmc/core/mmc.c | 75 ++ include/linux/mmc/card.h | 13 2 files changed, 62 insertions(+), 26 deletions(-) -- 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] ARM: mach-shmobile: use GPIO interrupt also for card eject on SDHI0 on mackerel
When we switch to transaction-based runtime PM on SDHI / TMIO MMC,
also card eject events will have to be detected by the platform.
This patch prepares mackerel to this switch.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
arch/arm/mach-shmobile/board-mackerel.c | 68 +++
1 files changed, 60 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-shmobile/board-mackerel.c
b/arch/arm/mach-shmobile/board-mackerel.c
index 1b30195..05f5fe7 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -45,6 +45,7 @@
#include linux/tca6416_keypad.h
#include linux/usb/r8a66597.h
#include linux/usb/renesas_usbhs.h
+#include linux/workqueue.h
#include video/sh_mobile_hdmi.h
#include video/sh_mobile_lcdc.h
@@ -990,7 +991,7 @@ static struct platform_device fsi_ak4643_device = {
/*
* The card detect pin of the top SD/MMC slot (CN7) is active low and is
- * connected to GPIO A22 of SH7372 (GPIO_PORT41).
+ * connected to GPIO A22 of SH7372 (GPIO_PORT41 / IRQ8).
*/
static int slot_cn7_get_cd(struct platform_device *pdev)
{
@@ -998,12 +999,49 @@ static int slot_cn7_get_cd(struct platform_device *pdev)
}
/* SDHI0 */
-static irqreturn_t mackerel_sdhi0_gpio_cd(int irq, void *arg)
+struct sdhi_card_detect {
+ struct delayed_work work;
+ struct sh_mobile_sdhi_info *info;
+ int gpio_irq;
+ int gpio_port;
+ int gpio_cd;
+};
+
+static void sdhi_cd_work(struct work_struct *work)
+{
+ struct sdhi_card_detect *cd = container_of(work, struct
sdhi_card_detect, work.work);
+ int ret;
+
+ if (cd-gpio_cd = 0)
+ gpio_free(cd-gpio_cd);
+ ret = gpio_request(cd-gpio_port, NULL);
+ if (!ret) {
+ gpio_direction_input(cd-gpio_port);
+ ret = gpio_get_value(cd-gpio_port);
+ gpio_free(cd-gpio_port);
+ if (ret)
+ /* No card */
+ irq_set_irq_type(cd-gpio_irq, IRQ_TYPE_EDGE_FALLING);
+ else
+ /* Card in the slot */
+ irq_set_irq_type(cd-gpio_irq, IRQ_TYPE_EDGE_RISING);
+ }
+ if (cd-gpio_cd = 0)
+ gpio_request(cd-gpio_cd, NULL);
+}
+
+static irqreturn_t mackerel_sdhi_gpio_cd(int irq, void *arg)
{
- struct device *dev = arg;
- struct sh_mobile_sdhi_info *info = dev-platform_data;
+ struct sdhi_card_detect *cd = arg;
+ struct sh_mobile_sdhi_info *info = cd-info;
struct tmio_mmc_data *pdata = info-pdata;
+ if (irq != cd-gpio_irq)
+ return IRQ_NONE;
+
+ irq_set_irq_type(irq, IRQ_TYPE_NONE);
+
+ schedule_delayed_work(cd-work, msecs_to_jiffies(200));
tmio_mmc_cd_wakeup(pdata);
return IRQ_HANDLED;
@@ -1015,6 +1053,14 @@ static struct sh_mobile_sdhi_info sdhi0_info = {
.tmio_caps = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
};
+static struct sdhi_card_detect sdhi0_cd = {
+ .work = __DELAYED_WORK_INITIALIZER(sdhi0_cd.work,
sdhi_cd_work),
+ .info = sdhi0_info,
+ .gpio_irq = evt2irq(0x3340),
+ .gpio_port = GPIO_PORT172,
+ .gpio_cd= GPIO_FN_SDHICD0,
+};
+
static struct resource sdhi0_resources[] = {
[0] = {
.name = SDHI0,
@@ -1092,7 +1138,7 @@ static struct platform_device sdhi1_device = {
/*
* The card detect pin of the top SD/MMC slot (CN23) is active low and is
- * connected to GPIO SCIFB_SCK of SH7372 (GPIO_PORT162).
+ * connected to GPIO SCIFB_SCK of SH7372 (GPIO_PORT162 / IRQ0).
*/
static int slot_cn23_get_cd(struct platform_device *pdev)
{
@@ -1500,12 +1546,18 @@ static void __init mackerel_init(void)
gpio_request(GPIO_FN_SDHID0_1, NULL);
gpio_request(GPIO_FN_SDHID0_0, NULL);
- ret = request_irq(evt2irq(0x3340), mackerel_sdhi0_gpio_cd,
- IRQF_TRIGGER_FALLING, sdhi0 cd, sdhi0_device.dev);
+ /*
+* If the driver probes with a card plugged in, the native SDHICD0 IRQ
+* will trigger, when the runtime PM brings the interface up, and the
+* card will be detected. This interrupt is needed if there is no card
+* during probing and runtime PM turns the interface power off.
+*/
+ ret = request_irq(sdhi0_cd.gpio_irq, mackerel_sdhi_gpio_cd,
+ IRQF_TRIGGER_FALLING, sdhi0 cd, sdhi0_cd);
if (!ret)
sdhi0_info.tmio_flags |= TMIO_MMC_HAS_COLD_CD;
else
- pr_err(Cannot get IRQ #%d: %d\n, evt2irq(0x3340), ret);
+ pr_err(Cannot get IRQ #%d: %d\n, sdhi0_cd.gpio_irq, ret);
#if !defined(CONFIG_MMC_SH_MMCIF) !defined(CONFIG_MMC_SH_MMCIF_MODULE)
/* enable SDHI1 */
--
1.7.2.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
Re: [PATCH v2] mmc: tmio: fix recursive spinlock, don't schedule with interrupts disabled
Hi Chris
On Wed, 13 Jul 2011, Chris Ball wrote:
Hi Guennadi,
On Mon, Jun 20 2011, Guennadi Liakhovetski wrote:
Calling mmc_request_done() under a spinlock with interrupts disabled
leads to a recursive spin-lock on request retry path and to
scheduling in atomic context. This patch fixes both these problems
by moving mmc_request_done() to the scheduler workqueue.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
This is a bug-fix: without it the system Oopses with LOCKDEP enabled, so,
it should really go in 3.0. OTOH it is pretty intrusine and non-trivial,
so, reviews and tests are highly appreciated! Also, unfortunately, I
wasn't able to test it well enough with SDIO, because the driver for the
only SDIO card, that I have, reproducibly crashes the kernel:
Having trouble working out how to apply this -- for example, in this hunk:
@@ -618,7 +631,8 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
if (ireg (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_CARD_INSERT |
TMIO_STAT_CARD_REMOVE);
- mmc_detect_change(host-mmc, msecs_to_jiffies(100));
+ if (!work_pending(host-mmc-detect.work))
+ mmc_detect_change(host-mmc,
msecs_to_jiffies(100));
}
/* CRC and other errors */
In mmc-next there's a goto out; after the mmc_detect_change call, which
looks like it's always been there. Am I missing a patch this depends on?
(It'd be a good time to get a full set of tmio patches for 3.1 pulled
together, if you can do that.)
Ok, if you don't mind, I'll get a couple of hours of sleep, and will reply
to you first thing in the morning:-)
Thanks
Guennadi
Thanks!
- Chris.
--
Chris Ball c...@laptop.org http://printf.net/
One Laptop Per Child
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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: pxav3 controller needs 32 bit ADMA addressing
On Tue, Jul 12, 2011 at 5:47 AM, Philip Rakity prak...@marvell.com wrote: enable the quirk. Best used in conjunction with patch downgrading ADMA to SDMA when transfer is not aligned. Signed-off-by: Philip Rakity prak...@marvell.com --- drivers/mmc/host/sdhci-pxav3.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c index 4198dbb..fc7e4a5 100644 --- a/drivers/mmc/host/sdhci-pxav3.c +++ b/drivers/mmc/host/sdhci-pxav3.c @@ -195,7 +195,8 @@ static int __devinit sdhci_pxav3_probe(struct platform_device *pdev) clk_enable(clk); host-quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL - | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC; + | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC + | SDHCI_QUIRK_32BIT_ADMA_SIZE; /* enable 1/8V DDR capable */ host-mmc-caps |= MMC_CAP_1_8V_DDR; -- 1.7.0.4 Acked-by: Zhangfei Gao zhangfei@marvell.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: mmcoops + mmcblk
Hi Simon.. I'm sorry. i didn't understand this mail. Do you want to implement something with mmcblk and mmc_oops? Can you explain to me for more information? Best Regards, Jaehoon Chung Simon Horman wrote: Hi, I am looking over the [RFC] mmcoops with panic/oops and is strikes me that although platform data is used to limit the area of the MMC device to which the oops is witten, the entire device has to belong (be bound to) the mmc_oops driver and as such no part of the device can be used for anything else. I am wondering if I am missing something? In particular, I would like to allow a single MMC chip to be used both by mmcblk (as a filesystem) and as mmc_oops (a small area reserved for writing oops messages). -- 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
