Re: [PATCH] mmc: mxs-mmc: add support for pre_req and post_req
On Wed, Apr 20, 2011 at 05:22:34PM +0200, Per Forlin wrote: [...] Do you have omap_hsmmc and mmci mmc_test result data to share? I keep the here: https://wiki.linaro.org/WorkingGroups/Kernel/Specs/StoragePerfMMC-async-req Actually, I have seen this page before. I was wondering if you have mmc_test raw data (test log of cases #37 ~ #40) to share. -- Regards, Shawn -- 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: mxs-mmc: add support for pre_req and post_req
On Wed, Apr 20, 2011 at 05:30:22PM +0200, Per Forlin wrote: [...] Remove dma_map and dma_unmap from your host driver and run the tests (obviously nonblocking and blocking will have the same results). If there is still no performance gain the cache penalty is very small on your platform and therefore nonblocking doesn't improve things much. Please let me know the result. Sorry, I could not understand. What's the point to run the test when the driver is even broken. The removal of dma_map_sg and dma_unmap_sg makes mxs-mmc host driver broken. -- Regards, Shawn -- 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: CMD23 plumbing and support patchset.
Hi Chris, On Wed, Apr 20, 2011 at 8:44 PM, Chris Ball c...@laptop.org wrote: Hi, On Sat, Apr 16 2011, Andrei Warkentin wrote: This is the third version of the CMD23 plumbing and host driver support patch set. Changes: 1) CMD23 support (used for features such as reliable writes) is decoupled from general multiblock trans through use of a quirk for affected cards. 2) Newer Sandisk MMC products are whitelisted along with some known good ones. All other MMC products do not use CMD23 for general transfers (seems like safest choice for now). SD products unaffected. Just gave this a try on SEM04G, which is in the whitelist, but didn't see a performance increase (or decrease). What are you using for a testcase? I'm a little wary of running mmc_test on it. :) Can you give me the manufacturing date? This is pretty interesting. You can see an improvement even if you do something like time block writes (O_DIRECT | O_SYNC, of course). But after all synthetic tests I measured the time it took to do 4 SQLite insertions and saw something like 30% improvement. A -- 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: CMD23 plumbing and support patchset.
On Thu, Apr 21, 2011 at 1:29 AM, Andrei Warkentin andr...@motorola.com wrote: Hi Chris, On Wed, Apr 20, 2011 at 8:44 PM, Chris Ball c...@laptop.org wrote: Hi, On Sat, Apr 16 2011, Andrei Warkentin wrote: This is the third version of the CMD23 plumbing and host driver support patch set. Changes: 1) CMD23 support (used for features such as reliable writes) is decoupled from general multiblock trans through use of a quirk for affected cards. 2) Newer Sandisk MMC products are whitelisted along with some known good ones. All other MMC products do not use CMD23 for general transfers (seems like safest choice for now). SD products unaffected. Just gave this a try on SEM04G, which is in the whitelist, but didn't see a performance increase (or decrease). What are you using for a testcase? I'm a little wary of running mmc_test on it. :) Can you give me the manufacturing date? This is pretty interesting. You can see an improvement even if you do something like time block writes (O_DIRECT | O_SYNC, of course). But after all synthetic tests I measured the time it took to do 4 SQLite insertions and saw something like 30% improvement. And you're trying this on SDHCI I presume, right? A -- 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/3 v3] MMC: add runtime and system power-management support to the MMCIF driver
Adding support for runtime power-management to the MMCIF driver allows
it to save power as long as no card is present. System-wide power
management has been verified with experimental PM patches on AP4-
based systems.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
v3: A further improvement over v2, but in any case waiting for a result of
the runtime PM vs. driver unbinding discussion:
http://thread.gmane.org/gmane.linux.kernel.mmc/7433/focus=7476
specific changes are:
(1) the patch to bus.c is dropped as wrong, instead, platform specific
runtime PM prototype code has been fixed
(2) consolidated calls to
pm_runtime_put_noidle();
pm_runtime_suspend();
to one call to
pm_runtime_put_sync()
(3) fixed probe() error path.
v2: With this patch and with the patch to mmc/core/bus.c, that I've sent a
couple of minutes ago
http://article.gmane.org/gmane.linux.kernel.mmc/7433
PM works correctly with all possible modprobe / rmmod, card-insert /
eject, and STR scenarios, that I could come up with. The part in
sh_mmcif_remove() is a bit rough though...
drivers/mmc/host/sh_mmcif.c | 69 --
1 files changed, 65 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index d3871b6..1889d64 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -29,6 +29,7 @@
#include linux/mmc/sh_mmcif.h
#include linux/pagemap.h
#include linux/platform_device.h
+#include linux/pm_runtime.h
#include linux/spinlock.h
#define DRIVER_NAMEsh_mmcif
@@ -173,6 +174,7 @@ struct sh_mmcif_host {
struct completion intr_wait;
enum mmcif_state state;
spinlock_t lock;
+ bool power;
/* DMA support */
struct dma_chan *chan_rx;
@@ -877,11 +879,21 @@ static void sh_mmcif_set_ios(struct mmc_host *mmc, struct
mmc_ios *ios)
if (ios-power_mode == MMC_POWER_UP) {
if (p-set_pwr)
p-set_pwr(host-pd, ios-power_mode);
+ if (!host-power) {
+ pm_runtime_get_sync(host-pd-dev);
+ host-power = true;
+ }
} else if (ios-power_mode == MMC_POWER_OFF || !ios-clock) {
/* clock stop */
sh_mmcif_clock_control(host, 0);
- if (ios-power_mode == MMC_POWER_OFF p-down_pwr)
- p-down_pwr(host-pd);
+ if (ios-power_mode == MMC_POWER_OFF) {
+ if (host-power) {
+ pm_runtime_put(host-pd-dev);
+ host-power = false;
+ }
+ if (p-down_pwr)
+ p-down_pwr(host-pd);
+ }
host-state = STATE_IDLE;
return;
}
@@ -1053,6 +1065,12 @@ static int __devinit sh_mmcif_probe(struct
platform_device *pdev)
sh_mmcif_sync_reset(host);
platform_set_drvdata(pdev, host);
+ pm_runtime_enable(pdev-dev);
+ host-power = false;
+ ret = pm_runtime_resume(pdev-dev);
+ if (ret 0)
+ goto clean_up2;
+
/* See if we also get DMA */
sh_mmcif_request_dma(host, pd);
@@ -1063,13 +1081,13 @@ static int __devinit sh_mmcif_probe(struct
platform_device *pdev)
ret = request_irq(irq[0], sh_mmcif_intr, 0, sh_mmc:error, host);
if (ret) {
dev_err(pdev-dev, request_irq error (sh_mmc:error)\n);
- goto clean_up2;
+ goto clean_up3;
}
ret = request_irq(irq[1], sh_mmcif_intr, 0, sh_mmc:int, host);
if (ret) {
free_irq(irq[0], host);
dev_err(pdev-dev, request_irq error (sh_mmc:int)\n);
- goto clean_up2;
+ goto clean_up3;
}
sh_mmcif_detect(host-mmc);
@@ -1079,7 +1097,12 @@ static int __devinit sh_mmcif_probe(struct
platform_device *pdev)
sh_mmcif_readl(host-addr, MMCIF_CE_VERSION) 0x);
return ret;
+clean_up3:
+ mmc_remove_host(mmc);
+ sh_mmcif_release_dma(host);
+ pm_runtime_suspend(pdev-dev);
clean_up2:
+ pm_runtime_disable(pdev-dev);
clk_disable(host-hclk);
clean_up1:
mmc_free_host(mmc);
@@ -1112,15 +1135,53 @@ static int __devexit sh_mmcif_remove(struct
platform_device *pdev)
clk_disable(host-hclk);
mmc_free_host(host-mmc);
+ pm_runtime_put_sync(pdev-dev);
+ pm_runtime_disable(pdev-dev);
+ pm_runtime_get_noresume(pdev-dev);
return 0;
}
+#ifdef CONFIG_PM
+static int sh_mmcif_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct sh_mmcif_host *host = platform_get_drvdata(pdev);
+ int ret = mmc_suspend_host(host-mmc);
+
+ if (!ret) {
+ sh_mmcif_writel(host-addr,
[PATCH] MMC: tmio: fix a recent regression: restore .set_ios(MMC_POWER_UP) handling
The aggressive clock gating for TMIO MMC patch has broken switching
interface power on, using MFD or platform callbacks. Restore the
ios-power_mode == MMC_POWER_UP ios-clock == 0 case handling.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
This one should go in 2.6.39
drivers/mmc/host/tmio_mmc_pio.c | 10 +-
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 6e3271d..f4fac9f 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -772,15 +772,15 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct
mmc_ios *ios)
tmio_mmc_set_clock(host, ios-clock);
/* Power sequence - OFF - UP - ON */
- if (ios-power_mode == MMC_POWER_OFF || !ios-clock) {
+ if (ios-power_mode == MMC_POWER_UP) {
+ /* power up SD bus */
+ if (host-set_pwr)
+ host-set_pwr(host-pdev, 1);
+ } else if (ios-power_mode == MMC_POWER_OFF || !ios-clock) {
/* power down SD bus */
if (ios-power_mode == MMC_POWER_OFF host-set_pwr)
host-set_pwr(host-pdev, 0);
tmio_mmc_clk_stop(host);
- } else if (ios-power_mode == MMC_POWER_UP) {
- /* power up SD bus */
- if (host-set_pwr)
- host-set_pwr(host-pdev, 1);
} else {
/* start bus clock */
tmio_mmc_clk_start(host);
--
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 http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] MMC: protect the tmio_mmc driver against a theoretical race
The MMC subsystem does not guarantee, that host driver .request() and
.set_ios() callbacks are serialised. Such concurrent calls, however,
do not have to be meaningfully supported, drivers just have to make
sure to avoid any severe problems.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
drivers/mmc/host/tmio_mmc.h |1 +
drivers/mmc/host/tmio_mmc_pio.c | 62 +++
2 files changed, 57 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 099ed49..f353624 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -19,6 +19,7 @@
#include linux/highmem.h
#include linux/mmc/tmio.h
#include linux/pagemap.h
+#include linux/spinlock.h
/* Definitions for values the CTRL_SDIO_STATUS register can take. */
#define TMIO_SDIO_STAT_IOIRQ 0x0001
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 62d37de..6e3271d 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -243,8 +243,12 @@ static void tmio_mmc_reset_work(struct work_struct *work)
spin_lock_irqsave(host-lock, flags);
mrq = host-mrq;
- /* request already finished */
- if (!mrq
+ /*
+* is request already finished? Since we use a non-blocking
+* cancel_delayed_work(), it can happen, that a .set_ios() call preempts
+* us, so, have to check for IS_ERR(host-mrq)
+*/
+ if (IS_ERR_OR_NULL(mrq)
|| time_is_after_jiffies(host-last_req_ts +
msecs_to_jiffies(2000))) {
spin_unlock_irqrestore(host-lock, flags);
@@ -264,16 +268,19 @@ static void tmio_mmc_reset_work(struct work_struct *work)
host-cmd = NULL;
host-data = NULL;
- host-mrq = NULL;
host-force_pio = false;
spin_unlock_irqrestore(host-lock, flags);
tmio_mmc_reset(host);
+ /* Ready for new calls */
+ host-mrq = NULL;
+
mmc_request_done(host-mmc, mrq);
}
+/* called with host-lock held, interrupts disabled */
static void tmio_mmc_finish_request(struct tmio_mmc_host *host)
{
struct mmc_request *mrq = host-mrq;
@@ -281,13 +288,15 @@ static void tmio_mmc_finish_request(struct tmio_mmc_host
*host)
if (!mrq)
return;
- host-mrq = NULL;
host-cmd = NULL;
host-data = NULL;
host-force_pio = false;
cancel_delayed_work(host-delayed_reset_work);
+ host-mrq = NULL;
+
+ /* FIXME: mmc_request_done() can schedule! */
mmc_request_done(host-mmc, mrq);
}
@@ -685,15 +694,27 @@ static int tmio_mmc_start_data(struct tmio_mmc_host *host,
static void tmio_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
{
struct tmio_mmc_host *host = mmc_priv(mmc);
+ unsigned long flags;
int ret;
- if (host-mrq)
+ spin_lock_irqsave(host-lock, flags);
+
+ if (host-mrq) {
pr_debug(request not null\n);
+ if (IS_ERR(host-mrq)) {
+ spin_unlock_irqrestore(host-lock, flags);
+ mrq-cmd-error = -EAGAIN;
+ mmc_request_done(mmc, mrq);
+ return;
+ }
+ }
host-last_req_ts = jiffies;
wmb();
host-mrq = mrq;
+ spin_unlock_irqrestore(host-lock, flags);
+
if (mrq-data) {
ret = tmio_mmc_start_data(host, mrq-data);
if (ret)
@@ -708,8 +729,8 @@ static void tmio_mmc_request(struct mmc_host *mmc, struct
mmc_request *mrq)
}
fail:
- host-mrq = NULL;
host-force_pio = false;
+ host-mrq = NULL;
mrq-cmd-error = ret;
mmc_request_done(mmc, mrq);
}
@@ -723,6 +744,29 @@ fail:
static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
{
struct tmio_mmc_host *host = mmc_priv(mmc);
+ unsigned long flags;
+
+ spin_lock_irqsave(host-lock, flags);
+ if (host-mrq) {
+ if (IS_ERR(host-mrq)) {
+ dev_dbg(host-pdev-dev,
+ %s.%d: concurrent .set_ios(), clk %u, mode
%u\n,
+ current-comm, task_pid_nr(current),
+ ios-clock, ios-power_mode);
+ host-mrq = ERR_PTR(-EINTR);
+ } else {
+ dev_dbg(host-pdev-dev,
+ %s.%d: CMD%u active since %lu, now %lu!\n,
+ current-comm, task_pid_nr(current),
+ host-mrq-cmd-opcode, host-last_req_ts,
jiffies);
+ }
+ spin_unlock_irqrestore(host-lock, flags);
+ return;
+ }
+
+ host-mrq = ERR_PTR(-EBUSY);
+
+ spin_unlock_irqrestore(host-lock, flags);
if (ios-clock)
[PATCH 2/2] MMC: add runtime and system power-management support to the TMIO MMCI driver
This patch adds a very crude runtime power-management to the TMIO MMC
driver. It only takes care to enable the hardware on probe() and keep
it enabled until remove(), at which time it disables it again. A
finer grained runtime PM would require implementing card detection
support on switched off interface.
System-wide power management has been verified with experimental PM
patches on AP4-based systems.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
This patch slightly changes driver's PM behaviour also on non-SDHI
systems, namely, it adds a reset call to the resume path. If this is
undesired, it can be easily moved to the SDHI-specific part. On SDHI it
helps to faster recover the controller and avoid failing MMC requests due
to timing out interrupts, and a repeated request retry from the core.
Runtime PM might change, if the core behaviour changes on driver unbind.
drivers/mmc/host/sh_mobile_sdhi.c |6
drivers/mmc/host/tmio_mmc.c | 11 ++
drivers/mmc/host/tmio_mmc.h |8 +
drivers/mmc/host/tmio_mmc_pio.c | 58 +++--
4 files changed, 73 insertions(+), 10 deletions(-)
diff --git a/drivers/mmc/host/sh_mobile_sdhi.c
b/drivers/mmc/host/sh_mobile_sdhi.c
index cc70123..f60e954 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -143,10 +143,16 @@ static int sh_mobile_sdhi_remove(struct platform_device
*pdev)
return 0;
}
+static const struct dev_pm_ops tmio_mmc_dev_pm_ops = {
+ .suspend = tmio_mmc_host_suspend,
+ .resume = tmio_mmc_host_resume,
+};
+
static struct platform_driver sh_mobile_sdhi_driver = {
.driver = {
.name = sh_mobile_sdhi,
.owner = THIS_MODULE,
+ .pm = tmio_mmc_dev_pm_ops,
},
.probe = sh_mobile_sdhi_probe,
.remove = __devexit_p(sh_mobile_sdhi_remove),
diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c
index 79c5684..be739f7 100644
--- a/drivers/mmc/host/tmio_mmc.c
+++ b/drivers/mmc/host/tmio_mmc.c
@@ -30,7 +30,7 @@ static int tmio_mmc_suspend(struct platform_device *dev,
pm_message_t state)
struct mmc_host *mmc = platform_get_drvdata(dev);
int ret;
- ret = mmc_suspend_host(mmc);
+ ret = tmio_mmc_host_suspend(dev-dev);
/* Tell MFD core it can disable us now.*/
if (!ret cell-disable)
@@ -46,15 +46,12 @@ static int tmio_mmc_resume(struct platform_device *dev)
int ret = 0;
/* Tell the MFD core we are ready to be enabled */
- if (cell-resume) {
+ if (cell-resume)
ret = cell-resume(dev);
- if (ret)
- goto out;
- }
- mmc_resume_host(mmc);
+ if (!ret)
+ ret = tmio_mmc_host_resume(dev-dev);
-out:
return ret;
}
#else
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index f353624..249c724 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -121,4 +121,12 @@ static inline void tmio_mmc_release_dma(struct
tmio_mmc_host *host)
}
#endif
+#ifdef CONFIG_PM
+int tmio_mmc_host_suspend(struct device *dev);
+int tmio_mmc_host_resume(struct device *dev);
+#else
+#define tmio_mmc_host_suspend NULL
+#define tmio_mmc_host_resume NULL
+#endif
+
#endif
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index f4fac9f..d1791ba 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -39,6 +39,7 @@
#include linux/module.h
#include linux/pagemap.h
#include linux/platform_device.h
+#include linux/pm_runtime.h
#include linux/scatterlist.h
#include linux/workqueue.h
#include linux/spinlock.h
@@ -884,12 +885,17 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host
**host,
else
mmc-ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+ pm_runtime_enable(pdev-dev);
+ ret = pm_runtime_resume(pdev-dev);
+ if (ret 0)
+ goto pm_disable;
+
tmio_mmc_clk_stop(_host);
tmio_mmc_reset(_host);
ret = platform_get_irq(pdev, 0);
if (ret 0)
- goto unmap_ctl;
+ goto pm_suspend;
_host-irq = ret;
@@ -900,7 +906,7 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host
**host,
ret = request_irq(_host-irq, tmio_mmc_irq, IRQF_DISABLED |
IRQF_TRIGGER_FALLING, dev_name(pdev-dev), _host);
if (ret)
- goto unmap_ctl;
+ goto pm_suspend;
spin_lock_init(_host-lock);
@@ -910,6 +916,9 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host
**host,
/* See if we also get DMA */
tmio_mmc_request_dma(_host, pdata);
+ /* We have to keep the device powered for its card detection to work */
+ pm_runtime_get_noresume(pdev-dev);
+
mmc_add_host(mmc);
Re: [PATCH 0/5] consolidate sdhci pltfm OF drivers and get them self registered
Hi Wolfram, Thanks for the review. On Tue, Apr 19, 2011 at 12:20:31PM +0200, Wolfram Sang wrote: [...] The approach seems sensible, so have a look at my (mostly minor) comments inside the patches. However, there is one bigger piece missing. You converted all the drivers which had a seperate source-file and hooked into sdhci-pltfm.c. However, those are only those users which need additional code to work around the quirks. There are also users which can take the plain pltfm-driver with a properly set platform_data (check the thread [PATCH] mmc: add SDHCI driver for STM platforms (V2) for an example). Those have to be converted, too. Even those drivers need pltfm-something.c to accommodate the platform_data, right? I think sdhci-dove.c (sitting on mainline) is also such an example. So if I'm not mistaken, I did take care of the drivers which can take the current plain pltfm-sdhci driver. Now the discussion could be if every of those users gets its own pltfm-something.c or if we create something similat to sdhci-pltfm-generic, which can also be setup with platform_data like the old driver (/me likes the latter a bit more. If we don't change the name of the driver (not talking about the sourcefile) and keep it sdhci-pltfm, then you wouldn't need to change all those users if you ensured it behaves the same. Since there are already pltfm-something.c to hold platform_data for those users anyway, it's not an argument here. Also, I think the next version of this series should have all makers of a sdhci-pltfm user CCed so we give them a chance to report breakage. Or donate acks or tested-by. Ok, will do. -- Regards, Shawn -- 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/5] mmc: sdhci: make sdhci-pltfm device drivers self registered
On Tue, Apr 19, 2011 at 12:20:42PM +0200, Wolfram Sang wrote:
On Fri, Mar 25, 2011 at 04:48:47PM +0800, Shawn Guo wrote:
The patch turns the common stuff in sdhci-pltfm.c into functions, and
add device drivers their own .probe and .remove which in turn call
into the common functions, so that those sdhci-pltfm device drivers
register itself and keep all device specific things away from common
sdhci-pltfm file.
Signed-off-by: Shawn Guo shawn@linaro.org
---
I'll second the comments from Grant (with one slight exception which is
noted below)
+static int __devexit sdhci_dove_remove(struct platform_device *pdev)
+{
+ struct sdhci_host *host = platform_get_drvdata(pdev);
+ int dead = 0;
+ u32 scratch;
+
+ scratch = readl(host-ioaddr + SDHCI_INT_STATUS);
+ if (scratch == (u32)-1)
+ dead = 1;
I'd prefer
dead = (readl() == 0x);
(or (u32)-1 if you prefer). But keeping a variable 'dead' is more
descriptive than keeping 'scratch'.
Ok.
+MODULE_LICENSE(GPL v2);
Just to be sure: Did you double-check if the original licenses were v2
or v2+?
It seems to me that sdhci-cns3xxx.c, sdhci-dove.c, sdhci-esdhc-imx.c
and sdhci-tegra.c all use v2.
Actually I do not even know how v2+ is stated. Do you have an example
for me to refer?
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
[...]
-err_add_host:
- if (pdata pdata-exit)
- pdata-exit(host);
-err_plat_init:
- iounmap(host-ioaddr);
err_remap:
release_mem_region(iomem-start, resource_size(iomem));
err_request:
sdhci_free_host(host);
err:
- printk(KERN_ERRProbing of sdhci-pltfm failed: %d\n, ret);
- return ret;
+ pr_err(%s failed %d\n, __func__, ret);
dev_err?
Good catch.
+ return NULL;
}
I didn't pay much attention to the OF version of the tegra driver, since
it still is not upstream yet, right?
Do not worry about that. You will not see that in the next version,
which will be based on mmc-next. And tegra OF support should be in
another patch.
--
Regards,
Shawn
--
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/5] mmc: sdhci: eliminate sdhci_of_host and sdhci_of_data
On Tue, Apr 19, 2011 at 12:20:53PM +0200, Wolfram Sang wrote:
On Fri, Mar 25, 2011 at 04:48:48PM +0800, Shawn Guo wrote:
The patch is to migrate the use of sdhci_of_host and sdhci_of_data
to sdhci_pltfm_host and sdhci_pltfm_data, so that the former pair can
be eliminated.
Signed-off-by: Shawn Guo shawn@linaro.org
---
[...]
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index a3e4be0..12afe86 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -19,6 +19,10 @@
struct sdhci_pltfm_host {
struct clk *clk;
u32 scratchpad; /* to handle quirks across io-accessor calls */
+
+ /* migrate from sdhci_of_host */
+ unsigned int clock;
+ u16 xfer_mode_shadow;
xfer_mode_shadow can be merged into scratchpad. They both fix the same
issue.
Yeah.
--
Regards,
Shawn
--
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] MMC: Block: Ensure hardware partitions don't mess with mmcblk device naming.
With the hardware partitions support (which represent additional logical devices
present on MMC), devidx does not correspond with index used to form
/dev/mmcblkX names. So use an additional allocated index for device names.
Signed-off-by: Andrei Warkentin andr...@motorola.com
---
drivers/mmc/card/block.c | 24 +---
1 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 9e30cf6..5572012 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -75,6 +75,7 @@ static int max_devices;
/* 256 minors, so at most 256 separate devices */
static DECLARE_BITMAP(dev_use, 256);
+static DECLARE_BITMAP(name_use, 256);
/*
* There is one mmc_blk_data per slot.
@@ -88,6 +89,7 @@ struct mmc_blk_data {
unsigned intusage;
unsigned intread_only;
unsigned intpart_type;
+ unsigned intname_idx;
/*
* Only set in main mmc_blk_data associated
@@ -776,6 +778,18 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct
mmc_card *card,
}
/*
+* !subname implies we are creating main mmc_blk_data that will be
+* associated with mmc_card with mmc_set_drvdata. Due to device
partitions,
+* devidx will not coincide with a per-physical card index anymore
+* so we keep track of a name index.
+*/
+ if (!subname)
+ md-name_idx = find_first_zero_bit(name_use, max_devices);
+ else
+ md-name_idx = ((struct mmc_blk_data *)
+ dev_to_disk(parent)-private_data)-name_idx;
+
+ /*
* Set the read-only status based on the supported commands
* and the write protect switch.
*/
@@ -820,13 +834,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct
mmc_card *card,
* messages to tell when the card is present.
*/
- if (subname)
- snprintf(md-disk-disk_name, sizeof(md-disk-disk_name),
-mmcblk%d%s,
-mmc_get_devidx(dev_to_disk(parent)), subname);
- else
- snprintf(md-disk-disk_name, sizeof(md-disk-disk_name),
-mmcblk%d, devidx);
+ snprintf(md-disk-disk_name, sizeof(md-disk-disk_name),
+mmcblk%d%s, md-name_idx, subname ? subname : );
blk_queue_logical_block_size(md-queue.queue, 512);
set_capacity(md-disk, size);
@@ -953,6 +962,7 @@ static void mmc_blk_remove_parts(struct mmc_card *card,
struct list_head *pos, *q;
struct mmc_blk_data *part_md;
+ __clear_bit(md-name_idx, name_use);
list_for_each_safe(pos, q, md-part) {
part_md = list_entry(pos, struct mmc_blk_data, part);
list_del(pos);
--
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 3/5] mmc: sdhci: make sdhci-of device drivers self registered
On Tue, Apr 19, 2011 at 12:21:01PM +0200, Wolfram Sang wrote:
+static int __devinit sdhci_esdhc_probe(struct platform_device *pdev)
+{
+ struct sdhci_host *host;
+ int ret;
+
+ host = sdhci_pltfm_init(pdev, sdhci_esdhc_pdata);
+ if (!host)
+ return -ENOMEM;
Just noticed: Since pltfm_init may fail due to various reasons, maybe
ERRPTR might be a good idea?
Ok.
[...]
+static int __init sdhci_hlwd_init(void)
+{
+ return platform_driver_register(sdhci_hlwd_driver);
+}
+module_init(sdhci_hlwd_init);
+
+static void __exit sdhci_hlwd_exit(void)
+{
+ platform_driver_unregister(sdhci_hlwd_driver);
+}
+module_exit(sdhci_hlwd_exit);
+
+MODULE_DESCRIPTION(Secure Digital Host Controller Interface OF driver);
+MODULE_AUTHOR(Xiaobo Xie x@freescale.com,
+ Anton Vorontsov avoront...@ru.mvista.com);
+MODULE_LICENSE(GPL v2);
Please double check the authors. It is based on the fsl driver, but the
copyright should go to
* Copyright (C) 2009 The GameCube Linux Team
* Copyright (C) 2009 Albert Herranz
I think.
Sorry, I misread the current copyright in sdhci-of-hlwd.c. You are
right.
--
Regards,
Shawn
--
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] MMC: Block: Ensure hardware partitions don't mess with mmcblk device naming.
On Thu, Apr 21, 2011 at 1:07 AM, Andrei Warkentin andr...@motorola.com wrote: With the hardware partitions support (which represent additional logical devices present on MMC), devidx does not correspond with index used to form /dev/mmcblkX names. So use an additional allocated index for device names. Signed-off-by: Andrei Warkentin andr...@motorola.com --- drivers/mmc/card/block.c | 24 +--- 1 files changed, 17 insertions(+), 7 deletions(-) The alternative is to borrow from a previous suggestion by Stephen Warren, but instead of using card-host-index in place of devidx, use card-host-index in place of an additional name_index used to form /dev/mmcblkX device names. Using card-host-index for devidx doesn't work when you have multiple logical devices per host (because MMC card contains HW partitions). Chris, unfortunately I am working OOF for next couple of days(I am happy I took my eMMC board with me), so I was not able to test second card (laptop only has 1 slot). Tested on your mmc tree with SDHCI and my MMC08G card. Would you mind testing above RFC change? Sorry again for the regression, g. A -- 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: mxs-mmc: add support for pre_req and post_req
On 21 April 2011 08:29, Shawn Guo shawn@freescale.com wrote: On Wed, Apr 20, 2011 at 05:30:22PM +0200, Per Forlin wrote: [...] Remove dma_map and dma_unmap from your host driver and run the tests (obviously nonblocking and blocking will have the same results). If there is still no performance gain the cache penalty is very small on your platform and therefore nonblocking doesn't improve things much. Please let me know the result. Sorry, I could not understand. What's the point to run the test when the driver is even broken. The removal of dma_map_sg and dma_unmap_sg makes mxs-mmc host driver broken. The point is only to get a measurement of the cost of handling dma_map_sg and dma_unmap_sg, this is the maximum time mmc nonblocking can save. The nonblocking mmc_test should save the total time of dma_map_sg and dma_unmap_sg, if the pre_req and post_req hooks are implemented correctly. Running without dma_map_sg and dma_unmap_sg will confirm if the pre_req and post_req hooks are implemented correctly. -- Regards, Shawn 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
[PATCH] sdhci: add quirk to support shared bus controller
From: Bin Shi bin@csr.com
some controllers share data bus or other pins between
multi-controllers and need to switch the functions of shared pins
runtime
this patch requested those shared pins before actual hardware access
and release them after access
Signed-off-by: Bin Shi bin@csr.com
Cc: Binghua Duan binghua.d...@csr.com
Signed-off-by: Barry Song 21cn...@gmail.com
---
drivers/mmc/host/sdhci.c | 13 +
drivers/mmc/host/sdhci.h |2 ++
include/linux/mmc/sdhci.h |2 ++
3 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index f31077d..7b07152 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1379,6 +1379,13 @@ static void sdhci_tasklet_finish(unsigned long param)
sdhci_reset(host, SDHCI_RESET_DATA);
}
+ /*
+* some controllers share data bus or other pins between
multi-controller
+* and need to switch the function of pins runtime
+*/
+ if (host-quirks SDHCI_QUIRK_SHARED_PINS)
+ host-ops-get_shared_pins(host);
+
host-mrq = NULL;
host-cmd = NULL;
host-data = NULL;
@@ -1391,6 +1398,12 @@ static void sdhci_tasklet_finish(unsigned long param)
spin_unlock_irqrestore(host-lock, flags);
mmc_request_done(host-mmc, mrq);
+
+ /*
+* release shared pins so that other controllers can use them
+*/
+ if (host-quirks SDHCI_QUIRK_SHARED_PINS)
+ host-ops-get_shared_pins(host);
}
static void sdhci_timeout_timer(unsigned long data)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 85750a9..9d918a5 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -229,6 +229,8 @@ struct sdhci_ops {
void (*platform_send_init_74_clocks)(struct sdhci_host *host,
u8 power_mode);
unsigned int(*get_ro)(struct sdhci_host *host);
+ unsigned int(*get_shared_pins)(struct sdhci_host *host);
+ unsigned int(*put_shared_pins)(struct sdhci_host *host);
};
#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 83bd9f7..32ab422 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -85,6 +85,8 @@ struct sdhci_host {
#define SDHCI_QUIRK_NO_HISPD_BIT (129)
/* Controller treats ADMA descriptors with length h incorrectly */
#define SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC (130)
+/* Controller shared data bus or other pins with other controllers */
+#define SDHCI_QUIRK_SHARED_PINS (131)
int irq;/* Device IRQ */
void __iomem *ioaddr; /* Mapped address */
--
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: mxs-mmc: add support for pre_req and post_req
On 21 April 2011 08:25, Shawn Guo shawn@freescale.com wrote: On Wed, Apr 20, 2011 at 05:22:34PM +0200, Per Forlin wrote: [...] Do you have omap_hsmmc and mmci mmc_test result data to share? I keep the here: https://wiki.linaro.org/WorkingGroups/Kernel/Specs/StoragePerfMMC-async-req Actually, I have seen this page before. I was wondering if you have mmc_test raw data (test log of cases #37 ~ #40) to share. My test numbers are 39 - 42, same tests though. PANDA - omap_hsmmc: mmc0: Test case 39. Multiple write performance with sync req 4k to 4MB... mmc0: Transfer of 32768 x 8 sectors (32768 x 4 KiB) took 18.987487792 seconds (7068 kB/s, 6903 KiB/s) mmc0: Transfer of 16384 x 16 sectors (16384 x 8 KiB) took 11.947631837 seconds (11233 kB/s, 10970 KiB/s) mmc0: Transfer of 8192 x 32 sectors (8192 x 16 KiB) took 9.472808839 seconds (14168 kB/s, 13836 KiB/s) mmc0: Transfer of 4096 x 64 sectors (4096 x 32 KiB) took 8.260650636 seconds (16247 kB/s, 15867 KiB/s) mmc0: Transfer of 2048 x 128 sectors (2048 x 64 KiB) took 7.622741699 seconds (17607 kB/s, 17194 KiB/s) mmc0: Transfer of 1024 x 256 sectors (1024 x 128 KiB) took 7.323181152 seconds (18327 kB/s, 17898 KiB/s) mmc0: Transfer of 512 x 512 sectors (512 x 256 KiB) took 6.744140626 seconds (19901 kB/s, 19434 KiB/s) mmc0: Transfer of 256 x 1024 sectors (256 x 512 KiB) took 6.679138184 seconds (20095 kB/s, 19624 KiB/s) mmc0: Transfer of 128 x 2048 sectors (128 x 1024 KiB) took 6.625518800 seconds (20257 kB/s, 19782 KiB/s) mmc0: Transfer of 32 x 8192 sectors (32 x 4096 KiB) took 6.620574950 seconds (20272 kB/s, 19797 KiB/s) mmc0: Result: OK mmc0: Tests completed. mmc0: Starting tests of card mmc0:80ca... mmc0: Test case 40. Multiple write performance with async req 4k to 4MB... mmc0: Transfer of 32768 x 8 sectors (32768 x 4 KiB) took 18.647644043 seconds (7197 kB/s, 7028 KiB/s) mmc0: Transfer of 16384 x 16 sectors (16384 x 8 KiB) took 11.624816894 seconds (11545 kB/s, 11275 KiB/s) mmc0: Transfer of 8192 x 32 sectors (8192 x 16 KiB) took 9.170440675 seconds (14635 kB/s, 14292 KiB/s) mmc0: Transfer of 4096 x 64 sectors (4096 x 32 KiB) took 7.965667725 seconds (16849 kB/s, 16454 KiB/s) mmc0: Transfer of 2048 x 128 sectors (2048 x 64 KiB) took 7.318023681 seconds (18340 kB/s, 17910 KiB/s) mmc0: Transfer of 1024 x 256 sectors (1024 x 128 KiB) took 7.040740969 seconds (19063 kB/s, 18616 KiB/s) mmc0: Transfer of 512 x 512 sectors (512 x 256 KiB) took 6.444641113 seconds (20826 kB/s, 20338 KiB/s) mmc0: Transfer of 256 x 1024 sectors (256 x 512 KiB) took 6.380249023 seconds (21036 kB/s, 20543 KiB/s) mmc0: Transfer of 128 x 2048 sectors (128 x 1024 KiB) took 6.43506 seconds (21192 kB/s, 20695 KiB/s) mmc0: Transfer of 32 x 8192 sectors (32 x 4096 KiB) took 6.328002930 seconds (21210 kB/s, 20713 KiB/s) mmc0: Result: OK mmc0: Tests completed. mmc0: Starting tests of card mmc0:80ca... mmc0: Test case 41. Multiple read performance with sync req 4k to 4MB... mmc0: Transfer of 32768 x 8 sectors (32768 x 4 KiB) took 20.567749024 seconds (6525 kB/s, 6372 KiB/s) mmc0: Transfer of 16384 x 16 sectors (16384 x 8 KiB) took 12.770507813 seconds (10509 kB/s, 10263 KiB/s) mmc0: Transfer of 8192 x 32 sectors (8192 x 16 KiB) took 10.003143311 seconds (13417 kB/s, 13103 KiB/s) mmc0: Transfer of 4096 x 64 sectors (4096 x 32 KiB) took 7.775665284 seconds (17261 kB/s, 16856 KiB/s) mmc0: Transfer of 2048 x 128 sectors (2048 x 64 KiB) took 6.960906982 seconds (19281 kB/s, 18829 KiB/s) mmc0: Transfer of 1024 x 256 sectors (1024 x 128 KiB) took 6.661651612 seconds (20147 kB/s, 19675 KiB/s) mmc0: Transfer of 512 x 512 sectors (512 x 256 KiB) took 6.510711672 seconds (20614 kB/s, 20131 KiB/s) mmc0: Transfer of 256 x 1024 sectors (256 x 512 KiB) took 6.434722900 seconds (20858 kB/s, 20369 KiB/s) mmc0: Transfer of 128 x 2048 sectors (128 x 1024 KiB) took 6.396850587 seconds (20981 kB/s, 20490 KiB/s) mmc0: Transfer of 32 x 8192 sectors (32 x 4096 KiB) took 6.368560791 seconds (21075 kB/s, 20581 KiB/s) mmc0: Result: OK mmc0: Tests completed. mmc0: Starting tests of card mmc0:80ca... mmc0: Test case 42. Multiple read performance with async req 4k to 4MB... mmc0: Transfer of 32768 x 8 sectors (32768 x 4 KiB) took 20.650848389 seconds (6499 kB/s, 6347 KiB/s) mmc0: Transfer of 16384 x 16 sectors (16384 x 8 KiB) took 12.796203613 seconds (10488 kB/s, 10243 KiB/s) mmc0: Transfer of 8192 x 32 sectors (8192 x 16 KiB) took 10.019592286 seconds (13395 kB/s, 13081 KiB/s) mmc0: Transfer of 4096 x 64 sectors (4096 x 32 KiB) took 7.603942870 seconds (17651 kB/s, 17237 KiB/s) mmc0: Transfer of 2048 x 128 sectors (2048 x 64 KiB) took 6.751251223 seconds (19880 kB/s, 19414 KiB/s) mmc0: Transfer of 1024 x 256 sectors (1024 x 128 KiB) took 6.252929687 seconds (21464 kB/s, 20961 KiB/s) mmc0: Transfer of 512 x 512 sectors (512 x 256 KiB) took 5.956726076 seconds (22532 kB/s, 22004 KiB/s) mmc0: Transfer of 256 x 1024 sectors (256 x 512 KiB) took 5.863586425
Re: [PATCH] sdhci: add quirk to support shared bus controller
Hi,
On Thu, Apr 21, 2011 at 3:51 AM, Barry Song 21cn...@gmail.com wrote:
From: Bin Shi bin@csr.com
some controllers share data bus or other pins between
multi-controllers and need to switch the functions of shared pins
runtime
this patch requested those shared pins before actual hardware access
and release them after access
Signed-off-by: Bin Shi bin@csr.com
Cc: Binghua Duan binghua.d...@csr.com
Signed-off-by: Barry Song 21cn...@gmail.com
---
drivers/mmc/host/sdhci.c | 13 +
drivers/mmc/host/sdhci.h | 2 ++
include/linux/mmc/sdhci.h | 2 ++
3 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index f31077d..7b07152 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1379,6 +1379,13 @@ static void sdhci_tasklet_finish(unsigned long param)
sdhci_reset(host, SDHCI_RESET_DATA);
}
+ /*
+ * some controllers share data bus or other pins between
multi-controller
+ * and need to switch the function of pins runtime
+ */
+ if (host-quirks SDHCI_QUIRK_SHARED_PINS)
+ host-ops-get_shared_pins(host);
+
Why in tasklet_finish? Why not in sdhci_request?
No need to waste a quirk flag. Just invoke if method is not NULL.
Also, I would assume host-ops-get_shared_pins would need to
synchronize with other SDHCI instances
it shares the pins with. Since you do it after the host-lock (as you
should), what happens if get_shared_pins needs to wait?
Can you show the rest (sdhci driver implementing these hooks)?
host-mrq = NULL;
host-cmd = NULL;
host-data = NULL;
@@ -1391,6 +1398,12 @@ static void sdhci_tasklet_finish(unsigned long param)
spin_unlock_irqrestore(host-lock, flags);
mmc_request_done(host-mmc, mrq);
+
+ /*
+ * release shared pins so that other controllers can use them
+ */
+ if (host-quirks SDHCI_QUIRK_SHARED_PINS)
+ host-ops-get_shared_pins(host);
}
static void sdhci_timeout_timer(unsigned long data)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 85750a9..9d918a5 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -229,6 +229,8 @@ struct sdhci_ops {
void (*platform_send_init_74_clocks)(struct sdhci_host *host,
u8 power_mode);
unsigned int (*get_ro)(struct sdhci_host *host);
+ unsigned int (*get_shared_pins)(struct sdhci_host *host);
+ unsigned int (*put_shared_pins)(struct sdhci_host *host);
};
#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 83bd9f7..32ab422 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -85,6 +85,8 @@ struct sdhci_host {
#define SDHCI_QUIRK_NO_HISPD_BIT (129)
/* Controller treats ADMA descriptors with length h incorrectly */
#define SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC (130)
+/* Controller shared data bus or other pins with other controllers */
+#define SDHCI_QUIRK_SHARED_PINS (131)
int irq; /* Device IRQ */
void __iomem *ioaddr; /* Mapped address */
--
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
--
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: mxs-mmc: add support for pre_req and post_req
On Thu, Apr 21, 2011 at 10:46:18AM +0200, Per Forlin wrote: On 21 April 2011 08:29, Shawn Guo shawn@freescale.com wrote: On Wed, Apr 20, 2011 at 05:30:22PM +0200, Per Forlin wrote: [...] Remove dma_map and dma_unmap from your host driver and run the tests (obviously nonblocking and blocking will have the same results). If there is still no performance gain the cache penalty is very small on your platform and therefore nonblocking doesn't improve things much. Please let me know the result. Sorry, I could not understand. What's the point to run the test when the driver is even broken. The removal of dma_map_sg and dma_unmap_sg makes mxs-mmc host driver broken. The point is only to get a measurement of the cost of handling dma_map_sg and dma_unmap_sg, this is the maximum time mmc nonblocking can save. The nonblocking mmc_test should save the total time of dma_map_sg and dma_unmap_sg, if the pre_req and post_req hooks are implemented correctly. Running without dma_map_sg and dma_unmap_sg will confirm if the pre_req and post_req hooks are implemented correctly. With dma_map_sg and dma_unmap_sg removed, the mmc_test gave very low numbers, though blocking and non-blocking numbers are same. Is it an indication that pre_req and post_req hooks are not implemented correctly? If yes, can you please help to catch the mistakes? Thanks. mc0: Test case 39. Read performance with blocking req 4k to 4MB... mmc0: Transfer of 32768 x 8 sectors (32768 x 4 KiB) took 56.875013015 seconds (2 359 kB/s, 2304 KiB/s, 576.14 IOPS) mmc0: Transfer of 16384 x 16 sectors (16384 x 8 KiB) took 47.407562500 seconds ( 2831 kB/s, 2764 KiB/s, 345.59 IOPS) mmc0: Transfer of 8192 x 32 sectors (8192 x 16 KiB) took 42.708718750 seconds (3 142 kB/s, 3068 KiB/s, 191.81 IOPS) mmc0: Transfer of 4096 x 64 sectors (4096 x 32 KiB) took 40.227125000 seconds (3 336 kB/s, 3258 KiB/s, 101.82 IOPS) mmc0: Transfer of 2048 x 128 sectors (2048 x 64 KiB) took 38.91575 seconds ( 3448 kB/s, 3368 KiB/s, 52.62 IOPS) mmc0: Transfer of 1024 x 256 sectors (1024 x 128 KiB) took 38.249562498 seconds (3509 kB/s, 3426 KiB/s, 26.77 IOPS) mmc0: Transfer of 512 x 512 sectors (512 x 256 KiB) took 37.912342548 seconds (3 540 kB/s, 3457 KiB/s, 13.50 IOPS) mmc0: Transfer of 256 x 1024 sectors (256 x 512 KiB) took 37.743876391 seconds ( 3556 kB/s, 3472 KiB/s, 6.78 IOPS) mmc0: Transfer of 128 x 2048 sectors (128 x 1024 KiB) took 37.658104019 seconds (3564 kB/s, 3480 KiB/s, 3.39 IOPS) mmc0: Transfer of 39 x 6630 sectors (39 x 3315 KiB) took 37.086429038 seconds (3 569 kB/s, 3486 KiB/s, 1.05 IOPS) mmc0: Result: OK mmc0: Test case 40. Read performance with none blocking req 4k to 4MB... mmc0: Transfer of 32768 x 8 sectors (32768 x 4 KiB) took 56.732932555 seconds (2 365 kB/s, 2310 KiB/s, 577.58 IOPS) mmc0: Transfer of 16384 x 16 sectors (16384 x 8 KiB) took 47.342812500 seconds ( 2835 kB/s, 2768 KiB/s, 346.07 IOPS) mmc0: Transfer of 8192 x 32 sectors (8192 x 16 KiB) took 42.673906250 seconds (3 145 kB/s, 3071 KiB/s, 191.96 IOPS) mmc0: Transfer of 4096 x 64 sectors (4096 x 32 KiB) took 40.208218750 seconds (3 338 kB/s, 3259 KiB/s, 101.86 IOPS) mmc0: Transfer of 2048 x 128 sectors (2048 x 64 KiB) took 38.90675 seconds ( 3449 kB/s, 3368 KiB/s, 52.63 IOPS) mmc0: Transfer of 1024 x 256 sectors (1024 x 128 KiB) took 38.24474 seconds (3509 kB/s, 3427 KiB/s, 26.77 IOPS) mmc0: Transfer of 512 x 512 sectors (512 x 256 KiB) took 37.909719946 seconds (3 540 kB/s, 3457 KiB/s, 13.50 IOPS) mmc0: Transfer of 256 x 1024 sectors (256 x 512 KiB) took 37.741834105 seconds ( 3556 kB/s, 3472 KiB/s, 6.78 IOPS) mmc0: Transfer of 128 x 2048 sectors (128 x 1024 KiB) took 37.657555456 seconds (3564 kB/s, 3480 KiB/s, 3.39 IOPS) mmc0: Transfer of 39 x 6630 sectors (39 x 3315 KiB) took 37.086351431 seconds (3 569 kB/s, 3486 KiB/s, 1.05 IOPS) mmc0: Result: OK mmc0: Tests completed. -- Regards, Shawn -- 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: mxs-mmc: add support for pre_req and post_req
On 21 April 2011 11:11, Shawn Guo shawn@freescale.com wrote: On Thu, Apr 21, 2011 at 10:46:18AM +0200, Per Forlin wrote: On 21 April 2011 08:29, Shawn Guo shawn@freescale.com wrote: On Wed, Apr 20, 2011 at 05:30:22PM +0200, Per Forlin wrote: [...] Remove dma_map and dma_unmap from your host driver and run the tests (obviously nonblocking and blocking will have the same results). If there is still no performance gain the cache penalty is very small on your platform and therefore nonblocking doesn't improve things much. Please let me know the result. Sorry, I could not understand. What's the point to run the test when the driver is even broken. The removal of dma_map_sg and dma_unmap_sg makes mxs-mmc host driver broken. The point is only to get a measurement of the cost of handling dma_map_sg and dma_unmap_sg, this is the maximum time mmc nonblocking can save. The nonblocking mmc_test should save the total time of dma_map_sg and dma_unmap_sg, if the pre_req and post_req hooks are implemented correctly. Running without dma_map_sg and dma_unmap_sg will confirm if the pre_req and post_req hooks are implemented correctly. With dma_map_sg and dma_unmap_sg removed, the mmc_test gave very low numbers, though blocking and non-blocking numbers are same. Is it an indication that pre_req and post_req hooks are not implemented correctly? I think you could get the same numbers for the nonblocking with dma_map and dma_unmap in place. If yes, can you please help to catch the mistakes? I will take a look. -- Regards, Shawn 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 1/5] mmc: sdhci: make sdhci-pltfm device drivers self registered
+MODULE_LICENSE(GPL v2); Just to be sure: Did you double-check if the original licenses were v2 or v2+? It seems to me that sdhci-cns3xxx.c, sdhci-dove.c, sdhci-esdhc-imx.c and sdhci-tegra.c all use v2. Actually I do not even know how v2+ is stated. Do you have an example for me to refer? Check davinci_mmc.c (or grep for 'any later version'). -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] mmc: mxs-mmc: add support for pre_req and post_req
On 21 April 2011 11:47, Per Forlin per.for...@linaro.org wrote:
On 21 April 2011 11:11, Shawn Guo shawn@freescale.com wrote:
On Thu, Apr 21, 2011 at 10:46:18AM +0200, Per Forlin wrote:
On 21 April 2011 08:29, Shawn Guo shawn@freescale.com wrote:
On Wed, Apr 20, 2011 at 05:30:22PM +0200, Per Forlin wrote:
[...]
Remove dma_map and dma_unmap from your host driver and run the tests
(obviously nonblocking and blocking will have the same results). If
there is still no performance gain the cache penalty is very small on
your platform and therefore nonblocking doesn't improve things much.
Please let me know the result.
Sorry, I could not understand. What's the point to run the test when
the driver is even broken. The removal of dma_map_sg and
dma_unmap_sg makes mxs-mmc host driver broken.
The point is only to get a measurement of the cost of handling
dma_map_sg and dma_unmap_sg, this is the maximum time mmc nonblocking
can save.
The nonblocking mmc_test should save the total time of dma_map_sg and
dma_unmap_sg, if the pre_req and post_req hooks are implemented
correctly.
Running without dma_map_sg and dma_unmap_sg will confirm if the
pre_req and post_req hooks are implemented correctly.
With dma_map_sg and dma_unmap_sg removed, the mmc_test gave very low
numbers, though blocking and non-blocking numbers are same. Is it
an indication that pre_req and post_req hooks are not implemented
correctly?
I think you could get the same numbers for the nonblocking with
dma_map and dma_unmap in place.
If yes, can you please help to catch the mistakes?
I will take a look.
I found something:
static void mxs_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
{
struct mxs_mmc_host *host = mmc_priv(mmc);
WARN_ON(host-mrq != NULL);
host-mrq = mrq;
mxs_mmc_start_cmd(host, mrq-cmd);
}
This is the execution flow:
pre_req()
mxs_mmc_request()
post_req()
wait_for_completion()
pre_req()
mxs_mmc_request() returns before the prepared value is used
post_req() will run dma_unmap and set the cookie to 0, this mean in
your case dma_unmap_sg will be called twice.
You need to store away the prepared data in mxs_mmc_request().
Look at my patch for mmci, function mmci_get_next_data. That function
deals with this issue.
I didn't see this issue when I only looked at the patch since no
changes are made in the request-function.
--
Regards,
Shawn
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 v6] mmc: Add mmc CMD+ACMD passthrough ioctl
W dniu 21 kwietnia 2011 07:11 użytkownik Arnd Bergmann a...@arndb.de napisał: On Wednesday 20 April 2011 21:46:04 Michał Mirosław wrote: 2011/4/20 Arnd Bergmann a...@arndb.de: No, please don't try to invent random new ways of doing this. Your example relies on the assumption that the task is calling the entry point for its native word size. Some architectures intentionally allow calling the 32 bit entry point from 64 bit tasks and vice versa, e.g. for user space emulators converting to a different ABI, and in that case is_compat_task() produces the wrong result. Don't ever rely on that. This doesn't make sense to me. If you call 32-bit entry point from 64-bit process, you can't reliably pass pointers through the call (unless you limit 64-bit process to 32-bit address space). Do you know a working example of something using this kind of cross-call? There are people that use 32 bit programs on x86_64 in 64 bit mode and switch on the ADDR_LIMIT_32BIT personality, IIRC. This gives you more registers and lets you do 64 bit arithmetic while not using any more memory to store long pointers. There are a few problems with this, and the new x32 ABI will make it cleaner. Ok. Thanks for the pointers. I'm okay with the anon union + ``compat_ptr(*(u32 *))`` part of your solution. If everyone else thinks it is reasonable, I'll submit a v7 with it. No need for a union or a ptr_size member in the struct. Just use a single __u64 and let the user cast the pointer to that. This will work on all architectures. Union is just hiding this cast (it will be done in kernel) and allows cleaner code for userspace (there's a single kernel and possibly multiple applications that will implement this call). As I explained, it doesn't work. Please read my earlier mails. If you're referring to your mail: Subject: Re: [PATCH v4] mmc: Add ioctl to let userspace apps send ACMDs Date: Wed, 13 Apr 2011 01:00:39 +0200 Message-Id: 201104130100.39810.a...@arndb.de Then I already provided an example of implementation that's independent of endianness and avoids casts on userspace. However, I still think it should be implemented in compat_ioctl() because compat_blkdev_ioctl() expects it. Either that, or I add to the big switch in compat_blkdev_driver_ioctl(), and spreading this change out to block/compat_ioctl.c does not seem like The Right Thing to me. Yes, you definitely need to fill the .compat_ioctl member. We don't want new entries in the switch statement, in particular none that are specific to a single driver. Hmm, you're right. fs/compat_ioctl.c falls back to plain .ioctl if .compat_ioctl == NULL. No, it doesn't. I'll need to look at the code again then. Best Regards, Michał Mirosław -- 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: TMIO: add runtime PM calls to global suspend() / redume() methods
The TMIO MMC driver cannot generally suspend itself at runtime even
with no card inserted, because otherwise it wouldn't be able to detect
new cards. But when the system goes down for a global suspend, we can
use runtime PM calls to let it activate platform-specific PM hooks,
e.g., to switch off respective power domains.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
drivers/mmc/host/tmio_mmc.h |2 ++
drivers/mmc/host/tmio_mmc_pio.c |6 ++
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 249c724..58138a2 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -52,6 +52,8 @@ struct tmio_mmc_host {
void (*set_pwr)(struct platform_device *host, int state);
void (*set_clk_div)(struct platform_device *host, int state);
+ int pm_error;
+
/* pio related stuff */
struct scatterlist *sg_ptr;
struct scatterlist *sg_orig;
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index d1791ba..26598f1 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -980,6 +980,8 @@ int tmio_mmc_host_suspend(struct device *dev)
if (!ret)
tmio_mmc_disable_mmc_irqs(host, TMIO_MASK_ALL);
+ host-pm_error = pm_runtime_put_sync(dev);
+
return ret;
}
EXPORT_SYMBOL(tmio_mmc_host_suspend);
@@ -987,6 +989,10 @@ EXPORT_SYMBOL(tmio_mmc_host_suspend);
int tmio_mmc_host_resume(struct device *dev)
{
struct mmc_host *mmc = dev_get_drvdata(dev);
+ struct tmio_mmc_host *host = mmc_priv(mmc);
+
+ if (!host-pm_error)
+ pm_runtime_get_sync(dev);
tmio_mmc_reset(mmc_priv(mmc));
--
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 http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] mmc: Add mmc CMD+ACMD passthrough ioctl
On Wednesday 20 April 2011, Chris Ball wrote: On Wed, Apr 20 2011, John Calixto wrote: Have you had the chance to look at v6 of this patch? If so, what do you think? Looks good to me. Since Arnd's been reviewing, it'd be nice to get a Reviewed-by: tag from him and acknowledgement that his concerns are all addressed, and then I'll merge it. Once the pointer passing is worked out, I think we have basically resolved the technical issues. One more thing that I just noticed: The __u64 data_ptr needs to be aligned, otherwise you get a problem on x86, which uses different alignment for 64 bit members in structures between 32 and 64 bit ABIs. Putting the 64 bit members first in the data structure, and adding padding at the end to have a size that is a multiple of 8 bytes should take care of this. I don't quite understand the timeout stuff in there, so I'm not sure how I'd fill these from an application like qemu that blindly passes down commands. I'd feel more comfortable if we didn't have to specify these in the interface, but I have no strict objection if you think that passing those is a reasonable requirement. The more important question that is still unresolved however is the nontechnical one: Do we want users to use this interface for DRM applications? I would still prefer having a more high-level interface for this, ideally something where we just export a block device for the encrypted partition, with an ioctl interface to do the authentication. Alternatively, I could imagine an ioctl interface that exports each SD security command as a separate ioctl command, including the block read/write ones. The information to do this seems to be available in http://issuu.com/sravan/docs/sd_card, which describes the ACMDs. It shouldn't be too hard to implement them in a high-level API. What I don't know is whether we have a clear policy about implementing SD standard interfaces in the kernel that are not part of the redacted specs. 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 v6] mmc: Add mmc CMD+ACMD passthrough ioctl
On Thursday 21 April 2011, Michał Mirosław wrote: Subject: Re: [PATCH v4] mmc: Add ioctl to let userspace apps send ACMDs Date: Wed, 13 Apr 2011 01:00:39 +0200 Message-Id: 201104130100.39810.a...@arndb.de Then I already provided an example of implementation that's independent of endianness and avoids casts on userspace. Yes, v4 got that aspect right, as far as I can tell, aside from an incorrect cast (u8* instead of u8 __user *). 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 v6] mmc: Add mmc CMD+ACMD passthrough ioctl
W dniu 21 kwietnia 2011 13:15 użytkownik Arnd Bergmann a...@arndb.de napisał:
On Thursday 21 April 2011, Michał Mirosław wrote:
Subject: Re: [PATCH v4] mmc: Add ioctl to let userspace apps send ACMDs
Date: Wed, 13 Apr 2011 01:00:39 +0200
Message-Id: 201104130100.39810.a...@arndb.de
Then I already provided an example of implementation that's
independent of endianness and avoids casts on userspace.
Yes, v4 got that aspect right, as far as I can tell, aside
from an incorrect cast (u8* instead of u8 __user *).
Just to have an understanding about the issue. In the mail I pointed
to, you argued that union will not work because of endianness
problems. I haven't seen other arguments against the union approach,
and I have provided a solution for this case.
BTW, kernel side can also avoid the cast if the union is extended with
32-bit field. This works because an address of a union is an address
of each of its fields. IOW all fields start at the same address
regardless if the they are 32 or 64 bits in size.
struct mmc_ioc_cmd {
union {
__u64 __data_ptr_storage64;
__u32 __data_ptr_storage32;
void __user *data_ptr;
};
...
};
[...]
static int mmc_blk_compat_ioctl(struct block_device *bdev, fmode_t mode,
unsigned int cmd, unsigned long arg)
{
struct mmc_ioc_cmd blk;
if (cmd != MMC_IOC_CMD)
return -EINVAL;
copy_from_user(compat_ptr(arg), blk) ...
blk.data_ptr = compat_ptr(blk.__data_ptr_storage32);
return mmc_blk_ioctl_cmd(bdev, blk);
}
Best Regards,
Michał Mirosław
--
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
[RESEND PATCH 0/4] mmc_spi: Add support for regulator framework
Hi, resending the patchset as I missed some recipients on the first round. This patchset has the purpose of adding support for the regulator framework to the mmc_spi driver. The first three patches are preparatory cleanups to make the forth one more straightforward. Maybe the fourth patch can be improved, I am open to any suggestions about it. These changes take strong inspiration from the pxamci driver; they have been tested on a Motorola A910, which uses a regulator to powerup the MMC card connected to the SPI bus, a test from a current user of the mmc_spi driver would not hurt just to be sure no regressions have been introduced. Thanks, Antonio Antonio Ospite (4): mmc_spi.c: factor out the check for power capability mmc_spi.c: factor out the SD card shutdown sequence mmc_spi.c: factor out a mmc_spi_setpower() function mmc_spi.c: add support for the regulator framework drivers/mmc/host/mmc_spi.c | 194 +--- 1 files changed, 129 insertions(+), 65 deletions(-) -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? -- 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
[RESEND PATCH 3/4] mmc_spi.c: factor out a mmc_spi_setpower() function
Factor out a mmc_spi_setpower() function so to make changing it more
elegant without adding too much stuff to mmc_spi_set_ios().
Signed-off-by: Antonio Ospite osp...@studenti.unina.it
---
drivers/mmc/host/mmc_spi.c | 43 +++
1 files changed, 31 insertions(+), 12 deletions(-)
diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index 9eae23c..0f3672d 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -1221,6 +1221,26 @@ static void mmc_spi_shutdownsequence(struct mmc_spi_host
*host)
}
}
+static inline int mmc_spi_setpower(struct mmc_spi_host *host,
+ unsigned char power_mode,
+ unsigned int vdd)
+{
+ /* switch power on/off if possible, accounting for
+* max 250msec powerup time if needed.
+*/
+ if (mmc_spi_canpower(host)) {
+ switch (power_mode) {
+ case MMC_POWER_OFF:
+ case MMC_POWER_UP:
+ host-pdata-setpower(host-spi-dev, vdd);
+ if (power_mode == MMC_POWER_UP)
+ msleep(host-powerup_msecs);
+ }
+ }
+
+ return 0;
+}
+
static char *mmc_powerstring(u8 power_mode)
{
switch (power_mode) {
@@ -1236,24 +1256,23 @@ static void mmc_spi_set_ios(struct mmc_host *mmc,
struct mmc_ios *ios)
struct mmc_spi_host *host = mmc_priv(mmc);
if (host-power_mode != ios-power_mode) {
+ int ret;
dev_dbg(host-spi-dev, mmc_spi: power %s (%d)%s\n,
mmc_powerstring(ios-power_mode),
ios-vdd,
mmc_spi_canpower(host) ? , can switch : );
- /* switch power on/off if possible, accounting for
-* max 250msec powerup time if needed.
-*/
- if (mmc_spi_canpower(host)) {
- switch (ios-power_mode) {
- case MMC_POWER_OFF:
- case MMC_POWER_UP:
- host-pdata-setpower(host-spi-dev,
- ios-vdd);
- if (ios-power_mode == MMC_POWER_UP)
- msleep(host-powerup_msecs);
- }
+ ret = mmc_spi_setpower(host, ios-power_mode, ios-vdd);
+ if (ret) {
+ dev_err(mmc_dev(mmc), unable to set power\n);
+ /*
+* The .set_ios() function in the mmc_host_ops
+* struct return void, and failing to set the
+* power should be rare so we print an error and
+* return here.
+*/
+ return;
}
/* See 6.4.1 in the simplified SD card physical spec 2.0 */
--
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
[RESEND PATCH 2/4] mmc_spi.c: factor out the SD card shutdown sequence
Factor out the SD card shutdown sequence to a dedicated
mmc_spi_shutdownsequence() function in order to make mmc_spi_set_ios()
more readable, and also for symmetry with mmc_spi_initsequence() which
is already a dedicated function.
Signed-off-by: Antonio Ospite osp...@studenti.unina.it
---
drivers/mmc/host/mmc_spi.c | 90 +++
1 files changed, 48 insertions(+), 42 deletions(-)
diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index 49f5725..9eae23c 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -1176,6 +1176,51 @@ static void mmc_spi_initsequence(struct mmc_spi_host
*host)
}
}
+/* See Section 6.4.2, in SD Simplified Physical Layer Specification 2.0
+ *
+ * If powering down, ground all card inputs to avoid power delivery from data
+ * lines! On a shared SPI bus, this will probably be temporary; 6.4.2 of
+ * the simplified SD spec says this must last at least 1msec.
+ *
+ * - Clock low means CPOL 0, e.g. mode 0
+ * - MOSI low comes from writing zero
+ * - Chipselect is usually active low...
+ */
+static void mmc_spi_shutdownsequence(struct mmc_spi_host *host)
+{
+ int mres;
+ u8 nullbyte = 0;
+
+ host-spi-mode = ~(SPI_CPOL|SPI_CPHA);
+ mres = spi_setup(host-spi);
+ if (mres 0)
+ dev_dbg(host-spi-dev,
+ switch to SPI mode 0 failed\n);
+
+ if (spi_write(host-spi, nullbyte, 1) 0)
+ dev_dbg(host-spi-dev,
+ put spi signals to low failed\n);
+
+ /*
+* Now clock should be low due to spi mode 0;
+* MOSI should be low because of written 0x00;
+* chipselect should be low (it is active low)
+* power supply is off, so now MMC is off too!
+*
+* FIXME no, chipselect can be high since the
+* device is inactive and SPI_CS_HIGH is clear...
+*/
+ msleep(10);
+ if (mres == 0) {
+ host-spi-mode |= (SPI_CPOL|SPI_CPHA);
+ mres = spi_setup(host-spi);
+ if (mres 0)
+ dev_dbg(host-spi-dev,
+ switch back to SPI mode 3
+failed\n);
+ }
+}
+
static char *mmc_powerstring(u8 power_mode)
{
switch (power_mode) {
@@ -1215,49 +1260,10 @@ static void mmc_spi_set_ios(struct mmc_host *mmc,
struct mmc_ios *ios)
if (ios-power_mode == MMC_POWER_ON)
mmc_spi_initsequence(host);
- /* If powering down, ground all card inputs to avoid power
-* delivery from data lines! On a shared SPI bus, this
-* will probably be temporary; 6.4.2 of the simplified SD
-* spec says this must last at least 1msec.
-*
-* - Clock low means CPOL 0, e.g. mode 0
-* - MOSI low comes from writing zero
-* - Chipselect is usually active low...
-*/
+ /* See 6.4.2 in the simplified SD card physical spec 2.0 */
if (mmc_spi_canpower(host)
- ios-power_mode == MMC_POWER_OFF) {
- int mres;
- u8 nullbyte = 0;
-
- host-spi-mode = ~(SPI_CPOL|SPI_CPHA);
- mres = spi_setup(host-spi);
- if (mres 0)
- dev_dbg(host-spi-dev,
- switch to SPI mode 0 failed\n);
-
- if (spi_write(host-spi, nullbyte, 1) 0)
- dev_dbg(host-spi-dev,
- put spi signals to low failed\n);
-
- /*
-* Now clock should be low due to spi mode 0;
-* MOSI should be low because of written 0x00;
-* chipselect should be low (it is active low)
-* power supply is off, so now MMC is off too!
-*
-* FIXME no, chipselect can be high since the
-* device is inactive and SPI_CS_HIGH is clear...
-*/
- msleep(10);
- if (mres == 0) {
- host-spi-mode |= (SPI_CPOL|SPI_CPHA);
- mres = spi_setup(host-spi);
- if (mres 0)
- dev_dbg(host-spi-dev,
- switch back to SPI mode 3
-failed\n);
- }
- }
+ ios-power_mode == MMC_POWER_OFF)
+ mmc_spi_shutdownsequence(host);
host-power_mode = ios-power_mode;
}
--
1.7.4.4
--
To unsubscribe from this list:
[RESEND PATCH 4/4] mmc_spi.c: add support for the regulator framework
Add support for powering up SD cards driven by regulators.
This makes the mmc_spi driver work also with the Motorola A910 phone.
Signed-off-by: Antonio Ospite osp...@studenti.unina.it
---
drivers/mmc/host/mmc_spi.c | 61 +++
1 files changed, 49 insertions(+), 12 deletions(-)
diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index 0f3672d..acc2ec8 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -31,6 +31,7 @@
#include linux/dma-mapping.h
#include linux/crc7.h
#include linux/crc-itu-t.h
+#include linux/regulator/consumer.h
#include linux/scatterlist.h
#include linux/mmc/host.h
@@ -150,11 +151,13 @@ struct mmc_spi_host {
*/
void*ones;
dma_addr_t ones_dma;
+
+ struct regulator*vcc;
};
static inline int mmc_spi_canpower(struct mmc_spi_host *host)
{
- return host-pdata host-pdata-setpower;
+ return (host-pdata host-pdata-setpower) || host-vcc;
}
//
@@ -1225,17 +1228,35 @@ static inline int mmc_spi_setpower(struct mmc_spi_host
*host,
unsigned char power_mode,
unsigned int vdd)
{
+ if (!mmc_spi_canpower(host))
+ return -ENOSYS;
+
/* switch power on/off if possible, accounting for
* max 250msec powerup time if needed.
*/
- if (mmc_spi_canpower(host)) {
- switch (power_mode) {
- case MMC_POWER_OFF:
- case MMC_POWER_UP:
+ switch (power_mode) {
+ case MMC_POWER_OFF:
+ if (host-vcc) {
+ int ret = mmc_regulator_set_ocr(host-mmc,
+ host-vcc, 0);
+ if (ret)
+ return ret;
+ } else {
+ host-pdata-setpower(host-spi-dev, vdd);
+ }
+ break;
+
+ case MMC_POWER_UP:
+ if (host-vcc) {
+ int ret = mmc_regulator_set_ocr(host-mmc,
+ host-vcc, vdd);
+ if (ret)
+ return ret;
+ } else {
host-pdata-setpower(host-spi-dev, vdd);
- if (power_mode == MMC_POWER_UP)
- msleep(host-powerup_msecs);
}
+ msleep(host-powerup_msecs);
+ break;
}
return 0;
@@ -1420,12 +1441,28 @@ static int mmc_spi_probe(struct spi_device *spi)
* and power switching gpios.
*/
host-pdata = mmc_spi_get_pdata(spi);
- if (host-pdata)
- mmc-ocr_avail = host-pdata-ocr_mask;
- if (!mmc-ocr_avail) {
- dev_warn(spi-dev, ASSUMING 3.2-3.4 V slot power\n);
- mmc-ocr_avail = MMC_VDD_32_33|MMC_VDD_33_34;
+#ifdef CONFIG_REGULATOR
+ host-vcc = regulator_get(mmc_dev(host-mmc), vmmc);
+
+ if (IS_ERR(host-vcc)) {
+ host-vcc = NULL;
+ } else {
+ host-mmc-ocr_avail = mmc_regulator_get_ocrmask(host-vcc);
+ if (host-pdata host-pdata-ocr_mask)
+ dev_warn(mmc_dev(host-mmc),
+ ocr_mask/setpower will not be used\n);
}
+#endif
+ if (host-vcc == NULL) {
+ /* fall-back to platform data */
+ if (host-pdata)
+ mmc-ocr_avail = host-pdata-ocr_mask;
+ if (!mmc-ocr_avail) {
+ dev_warn(spi-dev, ASSUMING 3.2-3.4 V slot power\n);
+ mmc-ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+ }
+ }
+
if (mmc_spi_canpower(host)) {
host-powerup_msecs = host-pdata-powerup_msecs;
if (!host-powerup_msecs || host-powerup_msecs 250)
--
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
[RESEND PATCH 1/4] mmc_spi.c: factor out the check for power capability
Factor out the 'canpower' condition into a dedicated function in order
to avoid repetition and to make changing the condition easier.
Signed-off-by: Antonio Ospite osp...@studenti.unina.it
---
drivers/mmc/host/mmc_spi.c | 18 ++
1 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index 7c1e16a..49f5725 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -152,6 +152,10 @@ struct mmc_spi_host {
dma_addr_t ones_dma;
};
+static inline int mmc_spi_canpower(struct mmc_spi_host *host)
+{
+ return host-pdata host-pdata-setpower;
+}
//
@@ -1187,19 +1191,16 @@ static void mmc_spi_set_ios(struct mmc_host *mmc,
struct mmc_ios *ios)
struct mmc_spi_host *host = mmc_priv(mmc);
if (host-power_mode != ios-power_mode) {
- int canpower;
-
- canpower = host-pdata host-pdata-setpower;
dev_dbg(host-spi-dev, mmc_spi: power %s (%d)%s\n,
mmc_powerstring(ios-power_mode),
ios-vdd,
- canpower ? , can switch : );
+ mmc_spi_canpower(host) ? , can switch : );
/* switch power on/off if possible, accounting for
* max 250msec powerup time if needed.
*/
- if (canpower) {
+ if (mmc_spi_canpower(host)) {
switch (ios-power_mode) {
case MMC_POWER_OFF:
case MMC_POWER_UP:
@@ -1223,7 +1224,8 @@ static void mmc_spi_set_ios(struct mmc_host *mmc, struct
mmc_ios *ios)
* - MOSI low comes from writing zero
* - Chipselect is usually active low...
*/
- if (canpower ios-power_mode == MMC_POWER_OFF) {
+ if (mmc_spi_canpower(host)
+ ios-power_mode == MMC_POWER_OFF) {
int mres;
u8 nullbyte = 0;
@@ -1399,7 +1401,7 @@ static int mmc_spi_probe(struct spi_device *spi)
dev_warn(spi-dev, ASSUMING 3.2-3.4 V slot power\n);
mmc-ocr_avail = MMC_VDD_32_33|MMC_VDD_33_34;
}
- if (host-pdata host-pdata-setpower) {
+ if (mmc_spi_canpower(host)) {
host-powerup_msecs = host-pdata-powerup_msecs;
if (!host-powerup_msecs || host-powerup_msecs 250)
host-powerup_msecs = 250;
@@ -1459,7 +1461,7 @@ static int mmc_spi_probe(struct spi_device *spi)
host-dma_dev ? : , no DMA,
(host-pdata host-pdata-get_ro)
? : , no WP,
- (host-pdata host-pdata-setpower)
+ mmc_spi_canpower(host)
? : , no poweroff,
(mmc-caps MMC_CAP_NEEDS_POLL)
? , cd polling : );
--
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
Re: [PATCH v6] mmc: Add mmc CMD+ACMD passthrough ioctl
On Thursday 21 April 2011, Michał Mirosław wrote:
W dniu 21 kwietnia 2011 13:15 użytkownik Arnd Bergmann a...@arndb.de
napisał:
static int mmc_blk_compat_ioctl(struct block_device *bdev, fmode_t mode,
unsigned int cmd, unsigned long arg)
{
struct mmc_ioc_cmd blk;
if (cmd != MMC_IOC_CMD)
return -EINVAL;
copy_from_user(compat_ptr(arg), blk) ...
blk.data_ptr = compat_ptr(blk.__data_ptr_storage32);
return mmc_blk_ioctl_cmd(bdev, blk);
}
Yes, this works, but it requires having a compat_ioctl() handler function that
knows about the data structure, which we generally try to avoid.
The same method would even work if you only had a pointer member in the
structure and did not even attempt to make the structure compatible.
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: [RESEND PATCH 4/4] mmc_spi.c: add support for the regulator framework
On Thu, Apr 21, 2011 at 02:27:53PM +0200, Antonio Ospite wrote:
+#ifdef CONFIG_REGULATOR
+ host-vcc = regulator_get(mmc_dev(host-mmc), vmmc);
+
+ if (IS_ERR(host-vcc)) {
+ host-vcc = NULL;
+ } else {
+ host-mmc-ocr_avail = mmc_regulator_get_ocrmask(host-vcc);
+ if (host-pdata host-pdata-ocr_mask)
+ dev_warn(mmc_dev(host-mmc),
+ ocr_mask/setpower will not be used\n);
}
+#endif
Why is this code conditional? The regulator API will stub itself out
(by returning a null pointer, which plays well with your use of null) if
it's disabled. I'm also not seeing any corresponding code to release
the regulator.
--
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] OF: add MMC binding helpers
Hi all, in working to add OF support to S3C MMC I discovered that some good code has been already written for the MMC-over-SPI driver. This is the first attempt to use it to build a common set of OF bindings for MMC drivers. Please tell me whether I'm on a good way or not. cheers, Domenico -[ Domenico Andreoli, aka cavok --[ http://cavokz.wordpress.com/gpgkey/ ---[ 3A0F 2F80 F79C 678A 8936 4FEE 0677 9033 A20E BC50 -- 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] Add GPIO DT support to s3c24xx
From: Domenico Andreoli cav...@gmail.com
Assign proper OF node (= with matching physical base address) to each
s3c24xx GPIO chip.
Signed-off-by: Domenico Andreoli cav...@gmail.com
---
arch/arm/plat-s3c24xx/gpiolib.c | 36
1 file changed, 36 insertions(+)
Index: b/arch/arm/plat-s3c24xx/gpiolib.c
===
--- a/arch/arm/plat-s3c24xx/gpiolib.c 2011-04-10 23:28:31.0 +0200
+++ b/arch/arm/plat-s3c24xx/gpiolib.c 2011-04-11 11:23:36.0 +0200
@@ -19,6 +19,8 @@
#include linux/ioport.h
#include linux/io.h
#include linux/gpio.h
+#include linux/of.h
+#include linux/of_address.h
#include plat/gpio-core.h
#include plat/gpio-cfg.h
@@ -210,6 +212,39 @@
},
};
+#ifdef CONFIG_OF_GPIO
+
+static void s3c24xx_attach_of_node(struct s3c_gpio_chip *chip)
+{
+ struct device_node *dn;
+ struct resource res;
+ const char *dt_compat;
+
+ if (chip-base == S3C2410_GPACON)
+ dt_compat = samsung,s3c2410-gpio-a;
+ else
+ dt_compat = samsung,s3c2410-gpio;
+
+ for_each_compatible_node(dn, NULL, dt_compat) {
+ if (of_address_to_resource(dn, 0, res) 0) {
+ printk(KERN_ERR %s: unable to translate DT address\n,
dn-full_name);
+ continue;
+ }
+
+ if (chip-base == (res.start - S3C24XX_PA_GPIO +
S3C24XX_VA_GPIO)) {
+ chip-chip.of_node = dn;
+ break;
+ }
+ }
+}
+
+#else
+
+static void s3c24xx_attach_of_node(struct s3c_gpio_chip *chip)
+{
+}
+
+#endif
static __init int s3c24xx_gpiolib_init(void)
{
@@ -220,6 +255,7 @@
if (!chip-config)
chip-config = s3c24xx_gpiocfg_default;
+ s3c24xx_attach_of_node(chip);
s3c_gpiolib_add(chip);
}
--
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: Add mmc CMD+ACMD passthrough ioctl
W dniu 21 kwietnia 2011 14:39 użytkownik Arnd Bergmann a...@arndb.de napisał:
On Thursday 21 April 2011, Michał Mirosław wrote:
W dniu 21 kwietnia 2011 13:15 użytkownik Arnd Bergmann a...@arndb.de
napisał:
static int mmc_blk_compat_ioctl(struct block_device *bdev, fmode_t mode,
unsigned int cmd, unsigned long arg)
{
struct mmc_ioc_cmd blk;
if (cmd != MMC_IOC_CMD)
return -EINVAL;
copy_from_user(compat_ptr(arg), blk) ...
blk.data_ptr = compat_ptr(blk.__data_ptr_storage32);
return mmc_blk_ioctl_cmd(bdev, blk);
}
Yes, this works, but it requires having a compat_ioctl() handler function that
knows about the data structure, which we generally try to avoid.
The same method would even work if you only had a pointer member in the
structure and did not even attempt to make the structure compatible.
Not really. If the structures were different, you would need write
code to translate it fully (this doesn't really apply for this simple
case, where there is only one pointer at the end of the structure). My
example only fixes/converts pointers in place.
The compat/native handler could be made in one function that takes
another bool arg and ioctl handlers would look like the code below. In
this scheme you avoid having to duplicate ioctl handler at all and let
compiler optimize out conditionals (you can even force
__mmc_blk_ioctl() inline to make sure of it).
#ifndef CONFIG_COMPAT
static inline void *compat_ptr(unsigned long) { BUG(); return NULL; }
#endif
static int __mmc_blk_ioctl(struct block_device *bdev, fmode_t mode,
unsigned int cmd, unsigned long arg, bool compat32)
{
void __user *p;
...
p = compat32 ? compat_ptr(arg) : (void *)arg;
...
if (compat32)
blk.data_ptr = compat_ptr(blk.__data_ptr_storage32);
...
}
static int mmc_blk_ioctl(struct block_device *bdev, fmode_t mode,
unsigned int cmd, unsigned long arg)
{
return __mmc_blk_ioctl(bdev, mode, cmd, arg, false);
}
#ifdef CONFIG_COMPAT
static int mmc_blk_compat_ioctl(struct block_device *bdev, fmode_t mode,
unsigned int cmd, unsigned long arg)
{
return __mmc_blk_ioctl(bdev, mode, cmd, arg, true);
}
#endif
--
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/2] Add OF binding helpers for MMC drivers
On Thu, Apr 21, 2011 at 03:19:57PM +0200, Domenico Andreoli wrote:
From: Domenico Andreoli cav...@gmail.com
This patch adds helpers to manage OF binding of MMC DeviceTree configs.
They don't cover all the MMC configuration cases, indeed are only a
slight generalization of those found in the MMC-over-SPI driver. More
will come later.
Can the mmc-over-spi driver be generalized to use this code too?
Signed-off-by: Domenico Andreoli cav...@gmail.com
---
drivers/of/Kconfig |4 +
drivers/of/Makefile|1 +
drivers/of/of_mmc.c| 165
+
include/linux/of_mmc.h | 50 +++
Personally I think it makes more sense for this code to live in
drivers/mmc/core.
4 files changed, 220 insertions(+)
Index: b/drivers/of/Kconfig
===
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -59,6 +59,10 @@ config OF_I2C
help
OpenFirmware I2C accessors
+config OF_MMC
+ depends on MMC
+ def_bool y
+
config OF_NET
depends on NETDEVICES
def_bool y
Index: b/drivers/of/Makefile
===
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_OF_DEVICE) += device.o plat
obj-$(CONFIG_OF_GPIO) += gpio.o
obj-$(CONFIG_OF_CLOCK) += clock.o
obj-$(CONFIG_OF_I2C) += of_i2c.o
+obj-$(CONFIG_OF_MMC) += of_mmc.o
obj-$(CONFIG_OF_NET) += of_net.o
obj-$(CONFIG_OF_SPI) += of_spi.o
obj-$(CONFIG_OF_MDIO)+= of_mdio.o
Index: b/drivers/of/of_mmc.c
===
--- /dev/null
+++ b/drivers/of/of_mmc.c
@@ -0,0 +1,165 @@
+/*
+ * OF helpers for the MMC API
+ *
+ * Copyright (c) 2011 Domenico Andreoli
+ *
+ * Heavily inspired by the OF support to the MMC-over-SPI driver made
+ * by Anton Vorontsov
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/device.h
+#include linux/err.h
+#include linux/slab.h
+#include linux/mmc/core.h
+#include linux/gpio.h
+#include linux/of.h
+#include linux/of_mmc.h
+#include linux/of_gpio.h
+
+static int of_read_mmc_gpio(struct of_mmc_crg *crg, int gpio_num)
+{
+ int value, active_low;
+
+ BUG_ON(gpio_num = NUM_MMC_GPIOS);
+
+ /* hitting this means that DeviceTree left this gpio unspecified
+ * by purpose but driver didn't take any measure to define its
+ * behavior (i.e. aborting probe phase or disabling the feature).
+ * driver needs to call of_is_valid_mmc_crg() for each expected
+ * gpio to detect this case.
+ */
+ if (WARN_ON(crg-gpios[gpio_num] 0))
+ return -1;
+
+ value = gpio_get_value(crg-gpios[gpio_num]);
+ active_low = crg-alow_gpios[gpio_num];
+ return value ^ active_low;
+}
+
+int of_get_mmc_cd_gpio(struct of_mmc_crg *crg)
+{
+ return of_read_mmc_gpio(crg, CD_MMC_GPIO);
+}
+EXPORT_SYMBOL(of_get_mmc_cd_gpio);
+
+int of_get_mmc_ro_gpio(struct of_mmc_crg *crg)
+{
+ return of_read_mmc_gpio(crg, WP_MMC_GPIO);
+}
+EXPORT_SYMBOL(of_get_mmc_ro_gpio);
+
+int of_is_valid_mmc_crg(struct of_mmc_crg *crg, int gpio_num)
+{
+ BUG_ON(gpio_num = NUM_MMC_GPIOS);
+ return gpio_is_valid(crg-gpios[gpio_num]);
+}
+EXPORT_SYMBOL(of_is_valid_mmc_crg);
+
+int of_get_mmc_crg(struct device *dev, struct device_node *np,
+ int cd_off, struct of_mmc_crg *crg)
+{
+ int *gpio, *alow;
+ int i, ret;
+
+ memset(crg, 0, sizeof(*crg));
+ crg-cd_irq = -1;
+
+ gpio = crg-gpios;
+ alow = crg-alow_gpios;
+ for (i = 0; i NUM_MMC_GPIOS; i++, gpio++, alow++) {
+ enum of_gpio_flags gpio_flags;
+ *gpio = of_get_gpio_flags(np, cd_off+i, gpio_flags);
+ *alow = !!(gpio_flags OF_GPIO_ACTIVE_LOW);
+
+ if (*gpio == -EEXIST || *gpio == -ENOENT) {
+ /* driver needs to define proper meaning of this missing
+gpio (i.e. abort probe or disable the feature) */
+ pr_debug(%s: gpio #%d is not specified\n, __func__,
i);
+ continue;
+ }
+ if (*gpio 0) {
+ pr_debug(%s: invalid configuration\n, __func__);
+ ret = *gpio;
+ break;
+ }
+ if (!gpio_is_valid(*gpio)) {
+ pr_debug(%s: gpio #%d is not valid: %d\n, __func__,
i, *gpio);
+ ret = -EINVAL;
+ break;
+ }
+ ret =
Re: [RFC] MMC: Block: Ensure hardware partitions don't mess with mmcblk device naming.
Hi Andrei, On Thu, Apr 21 2011, Andrei Warkentin wrote: Chris, unfortunately I am working OOF for next couple of days(I am happy I took my eMMC board with me), so I was not able to test second card (laptop only has 1 slot). Tested on your mmc tree with SDHCI and my MMC08G card. Would you mind testing above RFC change? Sorry again for the regression, g. Sure, happy to test. No need to feel too bad about the regression, we're very early on in preparation for .40. - 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: tmio: fix a recent regression: restore .set_ios(MMC_POWER_UP) handling
Hi,
On Thu, Apr 21 2011, Guennadi Liakhovetski wrote:
The aggressive clock gating for TMIO MMC patch has broken switching
interface power on, using MFD or platform callbacks. Restore the
ios-power_mode == MMC_POWER_UP ios-clock == 0 case handling.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
This one should go in 2.6.39
drivers/mmc/host/tmio_mmc_pio.c | 10 +-
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 6e3271d..f4fac9f 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -772,15 +772,15 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc,
struct mmc_ios *ios)
tmio_mmc_set_clock(host, ios-clock);
/* Power sequence - OFF - UP - ON */
- if (ios-power_mode == MMC_POWER_OFF || !ios-clock) {
+ if (ios-power_mode == MMC_POWER_UP) {
+ /* power up SD bus */
+ if (host-set_pwr)
+ host-set_pwr(host-pdev, 1);
+ } else if (ios-power_mode == MMC_POWER_OFF || !ios-clock) {
/* power down SD bus */
if (ios-power_mode == MMC_POWER_OFF host-set_pwr)
host-set_pwr(host-pdev, 0);
tmio_mmc_clk_stop(host);
- } else if (ios-power_mode == MMC_POWER_UP) {
- /* power up SD bus */
- if (host-set_pwr)
- host-set_pwr(host-pdev, 1);
} else {
/* start bus clock */
tmio_mmc_clk_start(host);
Thanks, pushed to mmc-next for .39.
Paul Parsons (CC'd) submitted a similar patch to fix the same regression.
Paul, please could you check that Guennadi's patch fixes your problem?
- 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: Fix read-only detection with JMicron 388 chip
On HP laptops with JMicron 388 chip, the write-locked SD card isn't
detected correctly as read-only in many cases. This is because the
PRESENT_STATE register becomes unsable just after plugging, and it
returns the WRITE_PROTECT bit wrongly at the first read.
This patch fixes the read-only detection by adding the own get_ro op
for checking the register more intensively with a relatively long
delay.
The patch is tested with 2.6.39-rc4 kernel.
Cc: Aries Lee aries...@jmicron.com
Signed-off-by: Takashi Iwai ti...@suse.de
---
drivers/mmc/host/sdhci-pci.c | 34 ++
1 files changed, 34 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
index a136be7..9037b2a 100644
--- a/drivers/mmc/host/sdhci-pci.c
+++ b/drivers/mmc/host/sdhci-pci.c
@@ -346,6 +346,35 @@ static void jmicron_enable_mmc(struct sdhci_host *host,
int on)
writeb(scratch, host-ioaddr + 0xC0);
}
+/* As PRESENT_STATE register becomes unstable just after plugging,
+ * need to check the RO-cap multiple times with a relatively long delay.
+ */
+
+#define SAMPLE_COUNT 5
+
+static unsigned int jmicron_get_ro(struct sdhci_host *host)
+{
+ int i, ro_count;
+
+ ro_count = 0;
+ for (i = 0; i SAMPLE_COUNT; i++) {
+ int present = sdhci_readl(host, SDHCI_PRESENT_STATE);
+ if (!(present SDHCI_WRITE_PROTECT)) {
+ if (++ro_count SAMPLE_COUNT / 2)
+ return 1;
+ }
+ msleep(30);
+ }
+ return 0;
+}
+
+static int sdhci_pci_enable_dma(struct sdhci_host *host);
+
+static struct sdhci_ops jmicron_pci_ops = {
+ .enable_dma = sdhci_pci_enable_dma,
+ .get_ro = jmicron_get_ro,
+};
+
static int jmicron_probe_slot(struct sdhci_pci_slot *slot)
{
if (slot-chip-pdev-revision == 0) {
@@ -383,6 +412,11 @@ static int jmicron_probe_slot(struct sdhci_pci_slot *slot)
slot-host-mmc-caps |= MMC_CAP_BUS_WIDTH_TEST;
+ /* Replace the ops for unstable get_ro detection */
+ if (slot-chip-pdev-device == PCI_DEVICE_ID_JMICRON_JMB388_SD ||
+ slot-chip-pdev-device == PCI_DEVICE_ID_JMICRON_JMB388_ESD)
+ slot-host-ops = jmicron_pci_ops;
+
return 0;
}
--
1.7.4.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: Fix read-only detection with JMicron 388 chip
Hi Takashi,
On Thu, Apr 21 2011, Takashi Iwai wrote:
On HP laptops with JMicron 388 chip, the write-locked SD card isn't
detected correctly as read-only in many cases. This is because the
PRESENT_STATE register becomes unsable just after plugging, and it
returns the WRITE_PROTECT bit wrongly at the first read.
This patch fixes the read-only detection by adding the own get_ro op
for checking the register more intensively with a relatively long
delay.
The patch is tested with 2.6.39-rc4 kernel.
Cc: Aries Lee aries...@jmicron.com
Signed-off-by: Takashi Iwai ti...@suse.de
---
drivers/mmc/host/sdhci-pci.c | 34 ++
1 files changed, 34 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
index a136be7..9037b2a 100644
--- a/drivers/mmc/host/sdhci-pci.c
+++ b/drivers/mmc/host/sdhci-pci.c
@@ -346,6 +346,35 @@ static void jmicron_enable_mmc(struct sdhci_host *host,
int on)
writeb(scratch, host-ioaddr + 0xC0);
}
+/* As PRESENT_STATE register becomes unstable just after plugging,
+ * need to check the RO-cap multiple times with a relatively long delay.
+ */
+
+#define SAMPLE_COUNT 5
+
+static unsigned int jmicron_get_ro(struct sdhci_host *host)
+{
+ int i, ro_count;
+
+ ro_count = 0;
+ for (i = 0; i SAMPLE_COUNT; i++) {
+ int present = sdhci_readl(host, SDHCI_PRESENT_STATE);
+ if (!(present SDHCI_WRITE_PROTECT)) {
+ if (++ro_count SAMPLE_COUNT / 2)
+ return 1;
+ }
+ msleep(30);
+ }
+ return 0;
+}
+
+static int sdhci_pci_enable_dma(struct sdhci_host *host);
+
+static struct sdhci_ops jmicron_pci_ops = {
+ .enable_dma = sdhci_pci_enable_dma,
+ .get_ro = jmicron_get_ro,
+};
+
static int jmicron_probe_slot(struct sdhci_pci_slot *slot)
{
if (slot-chip-pdev-revision == 0) {
@@ -383,6 +412,11 @@ static int jmicron_probe_slot(struct sdhci_pci_slot
*slot)
slot-host-mmc-caps |= MMC_CAP_BUS_WIDTH_TEST;
+ /* Replace the ops for unstable get_ro detection */
+ if (slot-chip-pdev-device == PCI_DEVICE_ID_JMICRON_JMB388_SD ||
+ slot-chip-pdev-device == PCI_DEVICE_ID_JMICRON_JMB388_ESD)
+ slot-host-ops = jmicron_pci_ops;
+
return 0;
}
I don't like overwriting ops here -- it's too magical, and now we have
to maintain the ops table in two places. A quirk seems justified here,
even though we're trying to reduce them in general. Can anyone find a
better solution?
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: Fix read-only detection with JMicron 388 chip
At Thu, 21 Apr 2011 12:40:26 -0400,
Chris Ball wrote:
Hi Takashi,
On Thu, Apr 21 2011, Takashi Iwai wrote:
On HP laptops with JMicron 388 chip, the write-locked SD card isn't
detected correctly as read-only in many cases. This is because the
PRESENT_STATE register becomes unsable just after plugging, and it
returns the WRITE_PROTECT bit wrongly at the first read.
This patch fixes the read-only detection by adding the own get_ro op
for checking the register more intensively with a relatively long
delay.
The patch is tested with 2.6.39-rc4 kernel.
Cc: Aries Lee aries...@jmicron.com
Signed-off-by: Takashi Iwai ti...@suse.de
---
drivers/mmc/host/sdhci-pci.c | 34 ++
1 files changed, 34 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
index a136be7..9037b2a 100644
--- a/drivers/mmc/host/sdhci-pci.c
+++ b/drivers/mmc/host/sdhci-pci.c
@@ -346,6 +346,35 @@ static void jmicron_enable_mmc(struct sdhci_host
*host, int on)
writeb(scratch, host-ioaddr + 0xC0);
}
+/* As PRESENT_STATE register becomes unstable just after plugging,
+ * need to check the RO-cap multiple times with a relatively long delay.
+ */
+
+#define SAMPLE_COUNT 5
+
+static unsigned int jmicron_get_ro(struct sdhci_host *host)
+{
+ int i, ro_count;
+
+ ro_count = 0;
+ for (i = 0; i SAMPLE_COUNT; i++) {
+ int present = sdhci_readl(host, SDHCI_PRESENT_STATE);
+ if (!(present SDHCI_WRITE_PROTECT)) {
+ if (++ro_count SAMPLE_COUNT / 2)
+ return 1;
+ }
+ msleep(30);
+ }
+ return 0;
+}
+
+static int sdhci_pci_enable_dma(struct sdhci_host *host);
+
+static struct sdhci_ops jmicron_pci_ops = {
+ .enable_dma = sdhci_pci_enable_dma,
+ .get_ro = jmicron_get_ro,
+};
+
static int jmicron_probe_slot(struct sdhci_pci_slot *slot)
{
if (slot-chip-pdev-revision == 0) {
@@ -383,6 +412,11 @@ static int jmicron_probe_slot(struct sdhci_pci_slot
*slot)
slot-host-mmc-caps |= MMC_CAP_BUS_WIDTH_TEST;
+ /* Replace the ops for unstable get_ro detection */
+ if (slot-chip-pdev-device == PCI_DEVICE_ID_JMICRON_JMB388_SD ||
+ slot-chip-pdev-device == PCI_DEVICE_ID_JMICRON_JMB388_ESD)
+ slot-host-ops = jmicron_pci_ops;
+
return 0;
}
I don't like overwriting ops here -- it's too magical, and now we have
to maintain the ops table in two places. A quirk seems justified here,
even though we're trying to reduce them in general.
Well, I also used quirk bit in my very first version I worked for
2.6.32 kernel. But when I looked at 2.6.39, quirks are almost full --
only the last one bit is left for bit 31. So I didn't want to finish
it :)
Can anyone find a better solution?
One way would be to copy the ops table itself in struct sdhci_host
instead of keeping the ops table pointer. Then you can overwrite only
the specific op in each probe_slot callback.
thanks,
Takashi
--
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: Fix read-only detection with JMicron 388 chip
Hi Takashi, On Thu, Apr 21 2011, Takashi Iwai wrote: I don't like overwriting ops here -- it's too magical, and now we have to maintain the ops table in two places. A quirk seems justified here, even though we're trying to reduce them in general. Well, I also used quirk bit in my very first version I worked for 2.6.32 kernel. But when I looked at 2.6.39, quirks are almost full -- only the last one bit is left for bit 31. So I didn't want to finish it :) Can anyone find a better solution? One way would be to copy the ops table itself in struct sdhci_host instead of keeping the ops table pointer. Then you can overwrite only the specific op in each probe_slot callback. I think I'd rather not touch ops at all and keep the code simple -- if you don't mind, please post a version with quirks, and I'll work on freeing up a few bits. 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/RFC] MMC: remove unbalanced pm_runtime_suspend()
On Thursday, April 21, 2011, Alan Stern wrote:
On Wed, 20 Apr 2011, Rafael J. Wysocki wrote:
On Wednesday, April 20, 2011, Alan Stern wrote:
On Wed, 20 Apr 2011, Rafael J. Wysocki wrote:
On Wednesday, April 20, 2011, Alan Stern wrote:
On Wed, 20 Apr 2011, Guennadi Liakhovetski wrote:
...
Ah, now I see the problem. It looks like we did not give sufficient
thought to the case where a device starts off (and therefore should
finish up) in a powered-down state. Calling pm_runtime_put_sync()
after unbinding the device driver seems a little futile -- with no
driver, the subsystem may not be able to power-down the device!
Rafael, how do you think we should handle this? Get rid of the
pm_runtime_get_no_resume() and pm_runtime_put_sync() calls in
dd.c:__device_release_driver()?
I think we need pm_runtime_barrier() in there. Otherwise we risk
removing the driver while there's a runtime PM request pending.
But we can move the pm_runtime_put_sync() before driver_sysfs_remove().
What happens if another runtime PM request is queued between the
put_sync() and the remove callback? We may need a safe way to prevent
async runtime PM requests while still allowing synchronous requests.
What about making a rule that it is invalid to schedule a future suspend
or queue a resume request of a device whose driver is being removed?
Arguably, we can't prevent people from shooting themselves in the foot this
way or another and I'm not sure if this particular case is worth additional
handling.
After thinking about this, I tend to agree. The synchronization
issues, combined with the unknown needs of the driver, make this very
difficult to handle in the PM core.
Here's another possible approach: If a driver wants to leave its device
in a powered-down state after unbinding then it can invoke its own
runtime_suspend callback directly, in the following way:
... unregister all child devices below dev ...
pm_runtime_disable(dev);
if (dev-power.runtime_status != RPM_SUSPENDED) {
pm_set_suspended(dev);
my_runtime_suspend_callback(dev);
}
I think this would work too, but then possibly many drivers would have to
do the same thing in their remove routines.
There may be issues regarding coordination with the subsystem or the
power domain; at the moment it's not clear what should be done. Maybe
the runtime-PM core should include an API for directly invoking the
appropriate callbacks.
If we choose this approach, then yes, we should provide a suitable API, but
I'm still thinking it would be simpler to move the pm_runtime_put_sync() before
driver_sysfs_remove() and make the rule as I said previously. :-)
Rafael
--
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: Fix read-only detection with JMicron 388 chip
At Thu, 21 Apr 2011 13:36:03 -0400, Chris Ball wrote: Hi Takashi, On Thu, Apr 21 2011, Takashi Iwai wrote: I don't like overwriting ops here -- it's too magical, and now we have to maintain the ops table in two places. A quirk seems justified here, even though we're trying to reduce them in general. Well, I also used quirk bit in my very first version I worked for 2.6.32 kernel. But when I looked at 2.6.39, quirks are almost full -- only the last one bit is left for bit 31. So I didn't want to finish it :) Can anyone find a better solution? One way would be to copy the ops table itself in struct sdhci_host instead of keeping the ops table pointer. Then you can overwrite only the specific op in each probe_slot callback. I think I'd rather not touch ops at all and keep the code simple -- if you don't mind, please post a version with quirks, and I'll work on freeing up a few bits. OK, I'll send shortly. Takashi -- 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/RFC] MMC: remove unbalanced pm_runtime_suspend()
On Thu, 21 Apr 2011, Rafael J. Wysocki wrote:
What about making a rule that it is invalid to schedule a future suspend
or queue a resume request of a device whose driver is being removed?
Arguably, we can't prevent people from shooting themselves in the foot
this
way or another and I'm not sure if this particular case is worth
additional
handling.
After thinking about this, I tend to agree. The synchronization
issues, combined with the unknown needs of the driver, make this very
difficult to handle in the PM core.
Here's another possible approach: If a driver wants to leave its device
in a powered-down state after unbinding then it can invoke its own
runtime_suspend callback directly, in the following way:
... unregister all child devices below dev ...
pm_runtime_disable(dev);
if (dev-power.runtime_status != RPM_SUSPENDED) {
pm_set_suspended(dev);
my_runtime_suspend_callback(dev);
}
I think this would work too, but then possibly many drivers would have to
do the same thing in their remove routines.
There may be issues regarding coordination with the subsystem or the
power domain; at the moment it's not clear what should be done. Maybe
the runtime-PM core should include an API for directly invoking the
appropriate callbacks.
If we choose this approach, then yes, we should provide a suitable API, but
I'm still thinking it would be simpler to move the pm_runtime_put_sync()
before driver_sysfs_remove() and make the rule as I said previously. :-)
The problem is synchronization. At what point is the driver supposed
to stop queuing runtime PM requests? It would have to be sometime
before the pm_runtime_barrier() call. How is the driver supposed to
know when that point is reached? The remove routine isn't called until
later.
Alan Stern
--
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/RFC] MMC: remove unbalanced pm_runtime_suspend()
On Thursday, April 21, 2011, Alan Stern wrote:
On Thu, 21 Apr 2011, Rafael J. Wysocki wrote:
What about making a rule that it is invalid to schedule a future suspend
or queue a resume request of a device whose driver is being removed?
Arguably, we can't prevent people from shooting themselves in the foot
this
way or another and I'm not sure if this particular case is worth
additional
handling.
After thinking about this, I tend to agree. The synchronization
issues, combined with the unknown needs of the driver, make this very
difficult to handle in the PM core.
Here's another possible approach: If a driver wants to leave its device
in a powered-down state after unbinding then it can invoke its own
runtime_suspend callback directly, in the following way:
... unregister all child devices below dev ...
pm_runtime_disable(dev);
if (dev-power.runtime_status != RPM_SUSPENDED) {
pm_set_suspended(dev);
my_runtime_suspend_callback(dev);
}
I think this would work too, but then possibly many drivers would have to
do the same thing in their remove routines.
There may be issues regarding coordination with the subsystem or the
power domain; at the moment it's not clear what should be done. Maybe
the runtime-PM core should include an API for directly invoking the
appropriate callbacks.
If we choose this approach, then yes, we should provide a suitable API, but
I'm still thinking it would be simpler to move the pm_runtime_put_sync()
before driver_sysfs_remove() and make the rule as I said previously. :-)
The problem is synchronization. At what point is the driver supposed
to stop queuing runtime PM requests? It would have to be sometime
before the pm_runtime_barrier() call. How is the driver supposed to
know when that point is reached? The remove routine isn't called until
later.
Executing the driver's callback is not an ideal solution either, because
it simply may be insufficient (it may be necessary to execute the power
domain and/or subsystem callbacks, pretty much what rpm_suspend() does,
but without taking the usage counter into consideration).
Moreover, if we want the driver's -remove() to do the cleanup anyway,
there's not much point in doing any cleanup before in the core. Also,
there's a little problem that the bus -remove() is called before the
driver's -remove(), so it may not be entirely possible to power down
the device when the driver's -remove() is called already.
I think the current code is better than any of the alternatives considered
so far.
Rafael
--
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/RFC] MMC: remove unbalanced pm_runtime_suspend()
On Thu, 21 Apr 2011, Rafael J. Wysocki wrote: If we choose this approach, then yes, we should provide a suitable API, but I'm still thinking it would be simpler to move the pm_runtime_put_sync() before driver_sysfs_remove() and make the rule as I said previously. :-) The problem is synchronization. At what point is the driver supposed to stop queuing runtime PM requests? It would have to be sometime before the pm_runtime_barrier() call. How is the driver supposed to know when that point is reached? The remove routine isn't called until later. Executing the driver's callback is not an ideal solution either, because it simply may be insufficient (it may be necessary to execute the power domain and/or subsystem callbacks, pretty much what rpm_suspend() does, but without taking the usage counter into consideration). That's why I suggested a new API. It could do the right callbacks. Moreover, if we want the driver's -remove() to do the cleanup anyway, there's not much point in doing any cleanup before in the core. Also, there's a little problem that the bus -remove() is called before the driver's -remove(), so it may not be entirely possible to power down the device when the driver's -remove() is called already. Actually, the bus-remove() callback (if there is one) is responsible for invoking the driver's callback. The subsystem should be smart enough to handle runtime PM requests while the driver's remove callback is running. I think the current code is better than any of the alternatives considered so far. Then you think Guennadi should leave his patch as it is, including the reversed put/get? Alan Stern -- 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/RFC] MMC: remove unbalanced pm_runtime_suspend()
On Thursday, April 21, 2011, Alan Stern wrote: On Thu, 21 Apr 2011, Rafael J. Wysocki wrote: If we choose this approach, then yes, we should provide a suitable API, but I'm still thinking it would be simpler to move the pm_runtime_put_sync() before driver_sysfs_remove() and make the rule as I said previously. :-) The problem is synchronization. At what point is the driver supposed to stop queuing runtime PM requests? It would have to be sometime before the pm_runtime_barrier() call. How is the driver supposed to know when that point is reached? The remove routine isn't called until later. Executing the driver's callback is not an ideal solution either, because it simply may be insufficient (it may be necessary to execute the power domain and/or subsystem callbacks, pretty much what rpm_suspend() does, but without taking the usage counter into consideration). That's why I suggested a new API. It could do the right callbacks. Moreover, if we want the driver's -remove() to do the cleanup anyway, there's not much point in doing any cleanup before in the core. Also, there's a little problem that the bus -remove() is called before the driver's -remove(), so it may not be entirely possible to power down the device when the driver's -remove() is called already. Actually, the bus-remove() callback (if there is one) is responsible for invoking the driver's callback. Ah, sorry, I misread the code in __device_release_driver() (too little coffee perhaps). The subsystem should be smart enough to handle runtime PM requests while the driver's remove callback is running. If we make such a rule, we may as well remove all of the runtime PM calls from __device_release_driver(). I think the current code is better than any of the alternatives considered so far. Then you think Guennadi should leave his patch as it is, including the reversed put/get? This, or we need to remove the runtime PM calls from __device_release_driver(). I'm a bit worried about the driver_sysfs_remove() and the bus notifier that in theory may affect the runtime PM callbacks potentially executed before -remove() is called. Rafael -- 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] MMC: Block: Ensure hardware partitions don't mess with mmcblk device naming.
Hi Andrei,
On Thu, Apr 21 2011, Andrei Warkentin wrote:
With the hardware partitions support (which represent additional logical
devices
present on MMC), devidx does not correspond with index used to form
/dev/mmcblkX names. So use an additional allocated index for device names.
Signed-off-by: Andrei Warkentin andr...@motorola.com
---
drivers/mmc/card/block.c | 24 +---
1 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 9e30cf6..5572012 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -75,6 +75,7 @@ static int max_devices;
/* 256 minors, so at most 256 separate devices */
static DECLARE_BITMAP(dev_use, 256);
+static DECLARE_BITMAP(name_use, 256);
/*
* There is one mmc_blk_data per slot.
@@ -88,6 +89,7 @@ struct mmc_blk_data {
unsigned intusage;
unsigned intread_only;
unsigned intpart_type;
+ unsigned intname_idx;
/*
* Only set in main mmc_blk_data associated
@@ -776,6 +778,18 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct
mmc_card *card,
}
/*
+ * !subname implies we are creating main mmc_blk_data that will be
+ * associated with mmc_card with mmc_set_drvdata. Due to device
partitions,
+ * devidx will not coincide with a per-physical card index anymore
+ * so we keep track of a name index.
+ */
+ if (!subname)
+ md-name_idx = find_first_zero_bit(name_use, max_devices);
+ else
+ md-name_idx = ((struct mmc_blk_data *)
+ dev_to_disk(parent)-private_data)-name_idx;
+
+ /*
* Set the read-only status based on the supported commands
* and the write protect switch.
*/
@@ -820,13 +834,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct
mmc_card *card,
* messages to tell when the card is present.
*/
- if (subname)
- snprintf(md-disk-disk_name, sizeof(md-disk-disk_name),
- mmcblk%d%s,
- mmc_get_devidx(dev_to_disk(parent)), subname);
- else
- snprintf(md-disk-disk_name, sizeof(md-disk-disk_name),
- mmcblk%d, devidx);
+ snprintf(md-disk-disk_name, sizeof(md-disk-disk_name),
+ mmcblk%d%s, md-name_idx, subname ? subname : );
blk_queue_logical_block_size(md-queue.queue, 512);
set_capacity(md-disk, size);
@@ -953,6 +962,7 @@ static void mmc_blk_remove_parts(struct mmc_card *card,
struct list_head *pos, *q;
struct mmc_blk_data *part_md;
+ __clear_bit(md-name_idx, name_use);
list_for_each_safe(pos, q, md-part) {
part_md = list_entry(pos, struct mmc_blk_data, part);
list_del(pos);
This is crashing for me on insertion of the external card after boot:
[2.479319] mmc2: new high speed MMC card at address 0001
[2.485055] mmcblk0: mmc2:0001 SEM04G 3.68 GiB
[2.489656] mmcblk0boot0: mmc2:0001 SEM04G partition 1 1.00 MiB
[2.495850] mmcblk0boot1: mmc2:0001 SEM04G partition 2 1.00 MiB
[2.503984] mmcblk0: p1 p2
[2.509273] mmcblk0boot1: unknown partition table
[2.515308] mmcblk0boot0: unknown partition table
[2.524044] psmouse.c: Failed to enable mouse on ec touchpad (phys)
[2.532834] EXT3-fs: barriers not enabled
[4.996102] EXT3-fs (mmcblk0p2): warning: maximal mount count reached,
running e2fsck is recommended
[5.007285] kjournald starting. Commit interval 5 seconds
[5.014595] EXT3-fs (mmcblk0p2): using internal journal
[5.019791] EXT3-fs (mmcblk0p2): recovery complete
[5.026713] EXT3-fs (mmcblk0p2): mounted filesystem with ordered data mode
[5.026725] VFS: Mounted root (ext3 filesystem) on device 179:2.
[5.039565] Freeing init memory: 116K
[olpc@localhost ~]$
[ 33.642250] Unable to handle kernel NULL pointer dereference at virtual
address 0021
[ 33.653175] pgd = c0004000
[ 33.656119] [0021] *pgd=
[ 33.659673] Internal error: Oops: 17 [#1] PREEMPT
[ 33.659673] last sysfs file:
/sys/devices/platform/sdhci-pxa.0/mmc_host/mmc0/mmc0:d555/serial
[ 33.672798] Modules linked in:
[ 33.675826] CPU: 0Tainted: GW
(2.6.39-rc4-00194-ge20de60-dirty #4)
[ 33.675826] PC is at sysfs_create_dir+0x24/0xd4
[ 33.683258] LR is at kobject_add_internal+0x134/0x224
[ 33.687761] pc : [c00f9880]lr : [c01b2600]psr: a013
[ 33.692773] sp : dc131db8 ip : fp :
[ 33.704168] r10: dcfe41fc r9 : c05571e0 r8 : dc205408
[ 33.704168] r7 : dcc61c68 r6 : r5 : dcc61c68 r4 : dcf441b0
[ 33.709357] r3 : dcc61c68 r2 : r1 : 002f r0 : dcf441b0
[ 33.715834] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment
kernel
[ 33.722316] Control: 10c5387d Table: 1cc44019 DAC: 0015
[ 33.729568]
v7 changelog for mmc ioctl patch
change history for this patch: - v7 - simplify 32-bit/64-bit buffer pointer portability - add pad member to struct mmc_ioc_cmd so its size is the same when built for either 32-bit or 64-bit. - register ``0xB3`` in Documentation/ioctl/ioctl-number.txt - v6 - refix 32+64 compat pointer for better portability - copy userspace pointer *before* using - apply upper limit to data buffer size - add flag to allow normal CMD opcodes as well as ACMD opcodes - remove unnecessary mutex grab - v5 - fix 32-bit compiler warning about the 32+64 compat pointer - v4 - replace postsleep udelay() with usleep_range() - add cmd_timeout_ms field for R1B commands - v3 - copy data from userspace before claiming host - break out copy from userspace into its own function - verify that caller has CAP_SYS_RAWIO - rename ``struct sd_ioc_cmd`` to ``struct mmc_ioc_cmd`` because it applies generally, not just to SD - make struct mmc_ioc_cmd the same between 32-bit and 64-bit to simplify compat_ioctl() - export include/linux/mmc/ioctl.h when you ``make headers_install`` - v2 - make initialization of struct declarations match kernel style - only allow ioctl() on whole block device, not partition - remove extraneous printks - implement compat_ioctl() - remove version field from ``struct sd_ioc_cmd`` John -- 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 v7] mmc: Add mmc CMD+ACMD passthrough ioctl
Allows appropriately-privileged applications to send CMD (normal) and
ACMD (application-specific; preceded with CMD55) commands to
cards/devices on the mmc bus. This is primarily useful for enabling the
security functionality built in to every SD card.
It can also be used as a generic passthrough (e.g. to enable virtual
machines to control mmc bus devices directly). However, this use case
has not been tested rigorously. Generic passthrough testing was only
conducted for a few non-security opcodes to prove the feasibility of the
passthrough.
Since any opcode can be sent using this passthrough, it is very possible
to render the card/device unusable. Applications that use this ioctl
must have CAP_SYS_RAWIO.
Security commands tested on TI PCIxx12 (SDHCI), Sigma Designs SMP8652
SoC, TI OMAP3621 SoC, TI OMAP3630 SoC, Samsung S5PC110 SoC, Qualcomm
MSM7200A SoC.
Signed-off-by: John Calixto john.cali...@modsystems.com
Reviewed-by: Andrei Warkentin andr...@motorola.com
---
Documentation/ioctl/ioctl-number.txt |1 +
drivers/mmc/card/block.c | 201 ++
drivers/mmc/core/sd_ops.c|3 +-
include/linux/Kbuild |1 +
include/linux/mmc/Kbuild |1 +
include/linux/mmc/core.h |1 +
include/linux/mmc/ioctl.h| 54 +
7 files changed, 261 insertions(+), 1 deletions(-)
create mode 100644 include/linux/mmc/Kbuild
create mode 100644 include/linux/mmc/ioctl.h
diff --git a/Documentation/ioctl/ioctl-number.txt
b/Documentation/ioctl/ioctl-number.txt
index a0a5d82..2a34d82 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -304,6 +304,7 @@ Code Seq#(hex) Include FileComments
0xB0 all RATIO devices in development:
mailto:v...@ratio.de
0xB1 00-1F PPPoX mailto:mostr...@styx.uwaterloo.ca
+0xB3 00 linux/mmc/ioctl.h
0xC0 00-0F linux/usb/iowarrior.h
0xCB 00-1F CBM serial IEC bus in development:
mailto:michael.kl...@puffin.lb.shuttle.de
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 61d233a..758419a 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -31,7 +31,11 @@
#include linux/mutex.h
#include linux/scatterlist.h
#include linux/string_helpers.h
+#include linux/delay.h
+#include linux/capability.h
+#include linux/compat.h
+#include linux/mmc/ioctl.h
#include linux/mmc/card.h
#include linux/mmc/host.h
#include linux/mmc/mmc.h
@@ -158,11 +162,208 @@ mmc_blk_getgeo(struct block_device *bdev, struct
hd_geometry *geo)
return 0;
}
+struct mmc_blk_ioc_data {
+ struct mmc_ioc_cmd ic;
+ unsigned char *buf;
+ u64 buf_bytes;
+};
+
+static struct mmc_blk_ioc_data *mmc_blk_ioctl_copy_from_user(
+ struct mmc_ioc_cmd __user *user)
+{
+ struct mmc_blk_ioc_data *idata;
+ int err;
+
+ idata = kzalloc(sizeof(*idata), GFP_KERNEL);
+ if (!idata) {
+ err = -ENOMEM;
+ goto copy_err;
+ }
+
+ if (copy_from_user(idata-ic, user, sizeof(idata-ic))) {
+ err = -EFAULT;
+ goto copy_err;
+ }
+
+ idata-buf_bytes = (u64) idata-ic.blksz * idata-ic.blocks;
+ if (idata-buf_bytes MMC_IOC_MAX_BYTES) {
+ err = -EOVERFLOW;
+ goto copy_err;
+ }
+
+ idata-buf = kzalloc(idata-buf_bytes, GFP_KERNEL);
+ if (!idata-buf) {
+ err = -ENOMEM;
+ goto copy_err;
+ }
+
+ if (copy_from_user(idata-buf, (void __user *)(unsigned long)
+ idata-ic.data_ptr, idata-buf_bytes)) {
+ err = -EFAULT;
+ goto copy_err;
+ }
+
+ return idata;
+
+copy_err:
+ kfree(idata-buf);
+ kfree(idata);
+ return ERR_PTR(err);
+
+}
+
+static int mmc_blk_ioctl_cmd(struct block_device *bdev,
+ struct mmc_ioc_cmd __user *ic_ptr)
+{
+ struct mmc_blk_ioc_data *idata;
+ struct mmc_blk_data *md;
+ struct mmc_card *card;
+ struct mmc_command cmd = {0};
+ struct mmc_data data = {0};
+ struct mmc_request mrq = {0};
+ struct scatterlist sg;
+ int err;
+
+ /*
+* The caller must have CAP_SYS_RAWIO, and must be calling this on the
+* whole block device, not on a partition. This prevents overspray
+* between sibling partitions.
+*/
+ if ((!capable(CAP_SYS_RAWIO)) || (bdev != bdev-bd_contains))
+ return -EPERM;
+
+ idata = mmc_blk_ioctl_copy_from_user(ic_ptr);
+ if (IS_ERR(idata))
+ return PTR_ERR(idata);
+
+ cmd.opcode = idata-ic.opcode;
+ cmd.arg = idata-ic.arg;
+ cmd.flags = idata-ic.flags;
+
+ data.sg = sg;
+ data.sg_len = 1;
+ data.blksz =
Re: [PATCH] MMC: TMIO: add runtime PM calls to global suspend() / redume() methods
Hi Guennadi,
On Thu, Apr 21, 2011 at 12:32:54PM +0200, Guennadi Liakhovetski wrote:
The TMIO MMC driver cannot generally suspend itself at runtime even
with no card inserted, because otherwise it wouldn't be able to detect
new cards. But when the system goes down for a global suspend, we can
use runtime PM calls to let it activate platform-specific PM hooks,
e.g., to switch off respective power domains.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
drivers/mmc/host/tmio_mmc.h |2 ++
drivers/mmc/host/tmio_mmc_pio.c |6 ++
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 249c724..58138a2 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -52,6 +52,8 @@ struct tmio_mmc_host {
void (*set_pwr)(struct platform_device *host, int state);
void (*set_clk_div)(struct platform_device *host, int state);
+ int pm_error;
I wonder if instead of adding an whole int to effectively
store one bit we could use some of the spare space in the
sdio_irq_enabled element, which is also an int that effectively
stores one bit.
Perhaps a bitmask or a bitfield?
I was thinking of doing something similar when adding support
for multiple IRQ vectors, as my current code also needs, wait for it,
one bit :-)
+
/* pio related stuff */
struct scatterlist *sg_ptr;
struct scatterlist *sg_orig;
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index d1791ba..26598f1 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -980,6 +980,8 @@ int tmio_mmc_host_suspend(struct device *dev)
if (!ret)
tmio_mmc_disable_mmc_irqs(host, TMIO_MASK_ALL);
+ host-pm_error = pm_runtime_put_sync(dev);
+
return ret;
}
EXPORT_SYMBOL(tmio_mmc_host_suspend);
@@ -987,6 +989,10 @@ EXPORT_SYMBOL(tmio_mmc_host_suspend);
int tmio_mmc_host_resume(struct device *dev)
{
struct mmc_host *mmc = dev_get_drvdata(dev);
+ struct tmio_mmc_host *host = mmc_priv(mmc);
+
+ if (!host-pm_error)
+ pm_runtime_get_sync(dev);
tmio_mmc_reset(mmc_priv(mmc));
--
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 http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7] mmc: Add mmc CMD+ACMD passthrough ioctl
On Thu, 21 Apr 2011, John Calixto wrote:
snip
+static struct mmc_blk_ioc_data *mmc_blk_ioctl_copy_from_user(
+ struct mmc_ioc_cmd __user *user)
+{
+ struct mmc_blk_ioc_data *idata;
+ int err;
+
+ idata = kzalloc(sizeof(*idata), GFP_KERNEL);
+ if (!idata) {
+ err = -ENOMEM;
+ goto copy_err;
+ }
+
+ if (copy_from_user(idata-ic, user, sizeof(idata-ic))) {
+ err = -EFAULT;
+ goto copy_err;
+ }
+
+ idata-buf_bytes = (u64) idata-ic.blksz * idata-ic.blocks;
+ if (idata-buf_bytes MMC_IOC_MAX_BYTES) {
+ err = -EOVERFLOW;
+ goto copy_err;
+ }
+
+ idata-buf = kzalloc(idata-buf_bytes, GFP_KERNEL);
+ if (!idata-buf) {
+ err = -ENOMEM;
+ goto copy_err;
+ }
+
Per Arnd's recommendation, I just cast the ``data_ptr`` to
``(void *)(unsigned long)`` to solve the compatibility issue with the
buffer pointer in struct mmc_ioc_cmd.
+ if (copy_from_user(idata-buf, (void __user *)(unsigned long)
+ idata-ic.data_ptr, idata-buf_bytes)) {
+ err = -EFAULT;
+ goto copy_err;
+ }
+
+ return idata;
+
+copy_err:
+ kfree(idata-buf);
+ kfree(idata);
+ return ERR_PTR(err);
+
+}
+
snip
I also left mmc_blk_ioctl() and mmc_blk_compat_ioctl() as they were in
v6. IMHO, a __mmc_blk_ioctl() function that accepts a compat32 boolean
arg does not do much to enhance readability or functionality in this
case. With the implementation in the 2 functions below, I intended to
make it clear that:
- mmc_blk_ioctl() is responsible for delegating to the appropriate
handler based on the incoming ioctl cmd. The ``if`` would be
replaced with a ``switch`` when adding other values for cmd.
- mmc_blk_compat_ioctl() is responsible for converting the ioctl arg
to something suitable for consumtption by mmc_blk_ioctl().
- when CONFIG_COMPAT is undefined, you get no code related to
compatibility.
+
+static int mmc_blk_ioctl(struct block_device *bdev, fmode_t mode,
+ unsigned int cmd, unsigned long arg)
+{
+ int ret = -EINVAL;
+ if (cmd == MMC_IOC_CMD)
+ ret = mmc_blk_ioctl_cmd(bdev, (struct mmc_ioc_cmd __user *)arg);
+ return ret;
+}
+
+#ifdef CONFIG_COMPAT
+static int mmc_blk_compat_ioctl(struct block_device *bdev, fmode_t mode,
+ unsigned int cmd, unsigned long arg)
+{
+ return mmc_blk_ioctl(bdev, mode, cmd, (unsigned long) compat_ptr(arg));
+}
+#endif
+
static const struct block_device_operations mmc_bdops = {
.open = mmc_blk_open,
.release= mmc_blk_release,
.getgeo = mmc_blk_getgeo,
.owner = THIS_MODULE,
+ .ioctl = mmc_blk_ioctl,
+#ifdef CONFIG_COMPAT
+ .compat_ioctl = mmc_blk_compat_ioctl,
+#endif
};
snip
It makes a lot of sense to me to make sure that struct mmc_ioc_cmd is
the same size on 32-bit and 64-bit machines, since it is used by _IOWR()
to derive the value for MMC_IOC_CMD. Any change in its size requires
having other compatibility logic to deal with a MMC_IOC_CMD32.
Note that as this struct has evolved, I also needed to add a ``__pad``
member to the struct to achieve the goal of keeping it the same size on
32-bit and 64-bit. I could have used packing or alignment compiler
directives, but this seemed simplest to me (I can add up the bytes in my
head!):
diff --git a/include/linux/mmc/ioctl.h b/include/linux/mmc/ioctl.h
new file mode 100644
index 000..225e1e1
--- /dev/null
+++ b/include/linux/mmc/ioctl.h
@@ -0,0 +1,54 @@
+#ifndef LINUX_MMC_IOCTL_H
+#define LINUX_MMC_IOCTL_H
+struct mmc_ioc_cmd {
+ /* Implies direction of data. true = write, false = read */
+ int write_flag;
+
+ /* Application-specific command. true = precede with CMD55 */
+ int is_acmd;
+
+ __u32 opcode;
+ __u32 arg;
+ __u32 response[4]; /* CMD response */
+ unsigned int flags;
+ unsigned int blksz;
+ unsigned int blocks;
+
+ /*
+ * Sleep at least postsleep_min_us useconds, and at most
+ * postsleep_max_us useconds *after* issuing command. Needed for some
+ * read commands for which cards have no other way of indicating
+ * they're ready for the next command (i.e. there is no equivalent of a
+ * busy indicator for read operations).
+ */
+ unsigned int postsleep_min_us;
+ unsigned int postsleep_max_us;
+
+ /*
+ * Override driver-computed timeouts. Note the difference in units!
+ */
+ unsigned int data_timeout_ns;
+ unsigned int cmd_timeout_ms;
+
+ /*
+ * For 64-bit machines, the next member, ``__u64 data_ptr``, wants to
+ * be 8-byte aligned. Make sure this struct is the same size when
+ * built for
Re: CMD23 plumbing and support patchset.
On Thu, Apr 21, 2011 at 1:30 AM, Andrei Warkentin andr...@motorola.com wrote: On Thu, Apr 21, 2011 at 1:29 AM, Andrei Warkentin andr...@motorola.com wrote: Hi Chris, On Wed, Apr 20, 2011 at 8:44 PM, Chris Ball c...@laptop.org wrote: Hi, On Sat, Apr 16 2011, Andrei Warkentin wrote: This is the third version of the CMD23 plumbing and host driver support patch set. Changes: 1) CMD23 support (used for features such as reliable writes) is decoupled from general multiblock trans through use of a quirk for affected cards. 2) Newer Sandisk MMC products are whitelisted along with some known good ones. All other MMC products do not use CMD23 for general transfers (seems like safest choice for now). SD products unaffected. Just gave this a try on SEM04G, which is in the whitelist, but didn't see a performance increase (or decrease). What are you using for a testcase? I'm a little wary of running mmc_test on it. :) Can you give me the manufacturing date? This is pretty interesting. You can see an improvement even if you do something like time block writes (O_DIRECT | O_SYNC, of course). But after all synthetic tests I measured the time it took to do 4 SQLite insertions and saw something like 30% improvement. And you're trying this on SDHCI I presume, right? Is this on the same board as the other log you sent? Controller is SDHCI-PXA? If you give me /sys/block/mmcblk0/device/date, I'll investigate why this improvement isn't helping you out. A -- 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: tmio: fix a recent regression: restore .set_ios(MMC_POWER_UP) handling
Yes, Guennadi's patch fixes my problem. As far as I can see the expression (ios-clock == 0 ios-power_mode == MMC_POWER_ON) will only be true if host-f_init == 0 at the time MMC_POWER_ON is invoked in mmc_power_up(). Presumably that case is intentional. --- On Thu, 21/4/11, Chris Ball c...@laptop.org wrote: Paul Parsons (CC'd) submitted a similar patch to fix the same regression. Paul, please could you check that Guennadi's patch fixes your problem? -- 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
