Re: [PATCH v9 11/12] mmc: core: add random fault injection
This patch prevents mmc core to be built as a module if mmc fault injection is selected. Changes needed to resolve this: * remove fault inject module from core.c * export should_fail() and init_fault_attr_dentries() in fault-injection.c * update documentation fault-injection.txt On 1 July 2011 18:55, Per Forlin per.for...@linaro.org wrote: This simple fault injection proved to be very useful to test the error handling in the block.c rw_rq(). It may still be useful to test if the host driver handle pre_req() and post_req() correctly in case of errors. Signed-off-by: Per Forlin per.for...@linaro.org Acked-by: Kyungmin Park kyungmin.p...@samsung.com Acked-by: Arnd Bergmann a...@arndb.de Reviewed-by: Venkatraman S svenk...@ti.com Tested-by: Sourav Poddarsourav.pod...@ti.com Tested-by: Linus Walleij linus.wall...@linaro.org --- drivers/mmc/core/core.c | 54 drivers/mmc/core/debugfs.c | 5 include/linux/mmc/host.h | 3 ++ lib/Kconfig.debug | 11 + 4 files changed, 73 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index d2d7239..62a8cc7 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -23,6 +23,8 @@ #include linux/log2.h #include linux/regulator/consumer.h #include linux/pm_runtime.h +#include linux/fault-inject.h +#include linux/random.h #include linux/mmc/card.h #include linux/mmc/host.h @@ -82,6 +84,56 @@ static void mmc_flush_scheduled_work(void) flush_workqueue(workqueue); } +#ifdef CONFIG_FAIL_MMC_REQUEST + +static DECLARE_FAULT_ATTR(fail_mmc_request); + +static int __init setup_fail_mmc_request(char *str) +{ + return setup_fault_attr(fail_mmc_request, str); +} +__setup(fail_mmc_request=, setup_fail_mmc_request); + +static void mmc_should_fail_request(struct mmc_host *host, + struct mmc_request *mrq) +{ + struct mmc_command *cmd = mrq-cmd; + struct mmc_data *data = mrq-data; + static const int data_errors[] = { + -ETIMEDOUT, + -EILSEQ, + -EIO, + }; + + if (!data) + return; + + if (cmd-error || data-error || !host-make_it_fail || + !should_fail(fail_mmc_request, data-blksz * data-blocks)) + return; + + data-error = data_errors[random32() % ARRAY_SIZE(data_errors)]; + data-bytes_xfered = (random32() % (data-bytes_xfered 9)) 9; +} + +static int __init fail_mmc_request_debugfs(void) +{ + return init_fault_attr_dentries(fail_mmc_request, + fail_mmc_request); +} + +late_initcall(fail_mmc_request_debugfs); This line prevents mmc core to be built as a module, since there will be two init_module (the mmc core one and this one) for the same module. + +#else /* CONFIG_FAIL_MMC_REQUEST */ + +static void mmc_should_fail_request(struct mmc_host *host, + struct mmc_request *mrq) +{ +} + +static int __init fail_mmc_request_debugfs(void) +{ +} Add fail_mmc_request_debugfs here and call it from mmc_init() +#endif /* CONFIG_FAIL_MMC_REQUEST */ + + /** * mmc_request_done - finish processing an MMC request * @host: MMC host which completed request @@ -108,6 +160,8 @@ void mmc_request_done(struct mmc_host *host, struct mmc_request *mrq) cmd-error = 0; host-ops-request(host, mrq); } else { + mmc_should_fail_request(host, mrq); + led_trigger_event(host-led, LED_OFF); pr_debug(%s: req done (CMD%u): %d: %08x %08x %08x %08x\n, diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c index 998797e..588e76f 100644 --- a/drivers/mmc/core/debugfs.c +++ b/drivers/mmc/core/debugfs.c @@ -188,6 +188,11 @@ void mmc_add_host_debugfs(struct mmc_host *host) root, host-clk_delay)) goto err_node; #endif +#ifdef CONFIG_FAIL_MMC_REQUEST + if (!debugfs_create_u8(make-it-fail, S_IRUSR | S_IWUSR, + root, host-make_it_fail)) + goto err_node; +#endif return; err_node: diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 59db6f2..0d0a48f 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -301,6 +301,9 @@ struct mmc_host { struct mmc_async_req *areq; /* active async req */ +#ifdef CONFIG_FAIL_MMC_REQUEST + u8 make_it_fail; +#endif unsigned long private[0] cacheline_aligned; }; diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index c768bcd..330fc70 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1057,6 +1057,17 @@ config FAIL_IO_TIMEOUT Only works with drivers that use the generic
Re: [PATCH v3 0/4] Add device tree probe for sdhci-esdhc-imx
On Tue, Jul 19, 2011 at 10:10:27AM +0200, Sascha Hauer wrote: On Mon, Jul 18, 2011 at 09:51:28PM -0400, Chris Ball wrote: Hi Shawn, Sascha, On Sat, Jul 09 2011, Shawn Guo wrote: Shawn Guo (4): mmc: sdhci-esdhc-imx: do not reference platform data after probe mmc: sdhci-esdhc-imx: get rid of the uses of cpu_is_mx() mmc: sdhci-pltfm: dt device does not pass parent to sdhci_alloc_host mmc: sdhci-esdhc-imx: add device tree probe support These don't apply against mmc-next -- what is your tree based on? It's based on linux-next with following patch applied. [PATCH v4 4/4] mmc: sdhci-esdhc-imx: extend card_detect and write_protect support for mx5 http://permalink.gmane.org/gmane.linux.kernel.mmc/8748 You applied the 3 of 4 patches in that series, and expect Sascha to send this one through his tree after you send those 3. Shawn, please ping me once all dependencies have been merged. I have no influence when my patches get merged since my patches go via Arnd. 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 2/3] eSDHC: Fix errors when booting kernel with fsl esdhc
On Mon, Jul 18, 2011 at 11:31 AM, Zang Roy-R61911 r61...@freescale.com wrote: -Original Message- From: S, Venkatraman [mailto:svenk...@ti.com] Sent: Tuesday, July 05, 2011 14:17 PM To: Zang Roy-R61911 Cc: linux-mmc; linuxppc-dev; cbouatmailru; akpm; Xu Lei-B33228; Kumar Gala Subject: Re: [PATCH 2/3] eSDHC: Fix errors when booting kernel with fsl esdhc On Tue, Jul 5, 2011 at 9:49 AM, Roy Zang tie-fei.z...@freescale.com wrote: From: Xu lei b33...@freescale.com When esdhc module was enabled in p5020, there were following errors: mmc0: Timeout waiting for hardware interrupt. mmc0: error -110 whilst initialising SD card mmc0: Unexpected interrupt 0x0200. mmc0: Timeout waiting for hardware interrupt. mmc0: error -110 whilst initialising SD card mmc0: Unexpected interrupt 0x0200. It is because ESDHC controller has different bit setting for PROCTL register, when kernel sets Power Control Register by method for standard SD Host Specification, it would overwritten FSL ESDHC PROCTL[DMAS]; when it set Host Control Registers[DMAS], it sets PROCTL[EMODE] and PROCTL[D3CD]. These operations will set bad bits for PROCTL Register on FSL ESDHC Controller and cause errors, so this patch will make esdhc driver access FSL PROCTL Register according to block guide instead of standard SD Host Specification. For some FSL chips, such as MPC8536/P2020, PROCTL[VOLT_SEL] and PROCTL[DMAS] bits are reserved and even if they are set to wrong bits there is no error. But considering that all FSL ESDHC Controller register map is not fully compliant to standard SD Host Specification, we put the patch to all of FSL ESDHC Controllers. Signed-off-by: Lei Xu b33...@freescale.com Signed-off-by: Roy Zang tie-fei.z...@freescale.com Signed-off-by: Kumar Gala ga...@kernel.crashing.org --- drivers/mmc/host/sdhci-of-core.c | 3 ++ drivers/mmc/host/sdhci.c | 62 ++- -- include/linux/mmc/sdhci.h | 6 ++- 3 files changed, 57 insertions(+), 14 deletions(-) [snip] diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 58d5436..77174e5 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -674,7 +674,7 @@ static void sdhci_set_transfer_irqs(struct sdhci_host *host) static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd) { u8 count; - u8 ctrl; + u32 ctrl; struct mmc_data *data = cmd-data; int ret; @@ -807,14 +807,28 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd) * is ADMA. */ if (host-version = SDHCI_SPEC_200) { - ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL); - ctrl = ~SDHCI_CTRL_DMA_MASK; - if ((host-flags SDHCI_REQ_USE_DMA) - (host-flags SDHCI_USE_ADMA)) - ctrl |= SDHCI_CTRL_ADMA32; - else - ctrl |= SDHCI_CTRL_SDMA; - sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); + if (host-quirks SDHCI_QUIRK_QORIQ_PROCTL_WEIRD) { +#define ESDHCI_PROCTL_DMAS_MASK 0x0300 +#define ESDHCI_PROCTL_ADMA32 0x0200 +#define ESDHCI_PROCTL_SDMA 0x Breaks the code flow / readability. Can be moved to top of the file ? The defines are only used in the following section. Why it will break the readability? I can also see this kind of define in the file ... #define SAMPLE_COUNT 5 static int sdhci_get_ro(struct mmc_host *mmc) ... Any rule should follow? [snip] @@ -1162,6 +1189,17 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned short power) host-pwr = pwr; + /* Now FSL ESDHC Controller has no Bus Power bit, + * and PROCTL[21] bit is for voltage selection */ Multiline comment style needed.. Will update. please help to explain your previous comment. Thanks. Roy There aren't very hard rules on this. Simple #defines are good, as a one off usage. These bit mask fields are very verbose, and they tend to grow more than a screenful. The remaining bits will never be defined ? -- 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/RFT] MMC: CORE: eMMC in Sleep mode before suspend
Hi, On Tue, Jul 19 2011, S, Venkatraman wrote: On Wed, Jul 13, 2011 at 8:06 PM, Balaji T K balaj...@ti.com wrote: Put MMC to sleep if it supports SLEEP/AWAKE (CMD5) in the mmc suspend to minimize power consumption. Signed-off-by: Balaji T K balaj...@ti.com Balaji, Would you mind reposting the patch without the RFC and s/CORE/core in subject line ? You can add my Acked-by: Venkatraman S svenk...@ti.com No need to resend, thanks -- pushed to mmc-next with these changes and the ACK. Anyone object to letting this soak in mmc-next for a release and merging it in 3.2? I'm worried that we'll find card or host quirks around this, and the 3.0 release is probably happening today. - Chris. -- Chris Ball c...@laptop.org http://printf.net/ One Laptop Per Child -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: Added quirks for Ricoh 1180:e823 lower base clock frequency
Here is flashbench data on Ricoh R5C822 for SanDisk SDSDXP1-016G-A75 16GB Extreme Pro SDHC Memory Card. Dell XPS 1330M (Ubuntu Natty). manjo@sleepy:~/Projects/flashbench$ sudo ./flashbench -a /dev/mmcblk0p1 [sudo] password for manjo: align 4294967296pre 1.91ms on 1.92ms post 1.92ms diff 4.46µs align 2147483648pre 1.99ms on 1.99ms post 1.99ms diff -642ns align 1073741824pre 1.99ms on 1.99ms post 1.99ms diff 435ns align 536870912 pre 1.99ms on 1.99ms post 1.99ms diff -1003ns align 268435456 pre 1.99ms on 1.99ms post 1.99ms diff -283ns align 134217728 pre 1.99ms on 1.99ms post 1.99ms diff 539ns align 67108864 pre 1.92ms on 1.91ms post 1.91ms diff 75ns align 33554432 pre 1.97ms on 2.01ms post 1.97ms diff 41µs align 16777216 pre 1.98ms on 2.02ms post 1.97ms diff 43.9µs align 8388608 pre 1.97ms on 1.97ms post 1.98ms diff -3481ns align 4194304 pre 2.22ms on 2.39ms post 1.97ms diff 292µs align 2097152 pre 2.29ms on 2.29ms post 2.3ms diff -3058ns align 1048576 pre 2.22ms on 2.22ms post 2.21ms diff 100ns align 524288pre 2.22ms on 2.21ms post 2.21ms diff -728ns align 262144pre 2.24ms on 2.24ms post 2.24ms diff 5.13µs align 131072pre 2.25ms on 2.24ms post 2.24ms diff 275ns align 65536 pre 2.23ms on 2.23ms post 2.23ms diff 1.7µs align 32768 pre 2.24ms on 2.23ms post 2.23ms diff -2799ns manjo@sleepy:~/Projects/flashbench$ sudo ./flashbench -O --erasesize=$[4 * 1024 * 1024] --blocksize=$[256 * 1024] /dev/mmcblk0p1 --open-au-nr=2 4MiB9.96M/s 2MiB11.2M/s 1MiB11.2M/s 512KiB 11.2M/s 256KiB 11.2M/s manjo@sleepy:~/Projects/flashbench$ Hi Manoj, On Mon, Jul 18 2011, Manoj Iyer wrote: Right, without the patch I get.. [ 52.526665] mmc0: new SDHC card at address e624 [ 52.571228] mmcblk0: mmc0:e624 SD16G 14.8 GiB [ 52.591071] mmcblk0: retrying using single block read [ 52.593105] mmcblk0: error -84 transferring data, sector 0, nr 8, card status 0x900 [ 52.593109] end_request: I/O error, dev mmcblk0, sector 0 [ 52.594594] mmcblk0: error -84 transferring data, sector 1, nr 7, card status 0x900 [ 52.594604] end_request: I/O error, dev mmcblk0, sector 1 [ 52.602893] quiet_error: 24 callbacks suppressed [ 52.602902] Buffer I/O error on device mmcblk0, logical block 0 [ 52.605349] ldm_validate_partition_table(): Disk read failed. [ 52.605384] Dev mmcblk0: unable to read RDB block 0 [ 52.607729] mmcblk0: unable to read partition table u@u:~$ So, I cannot generate any comparison data with this SD card. I see, thanks. So we're lacking any data on what speed the card would normally provide. Perhaps you could try that card on a different controller, just so we're able to see whether it's usually possible to get closer to 45M/sec with it? I think I'll take your patch as-is for 3.1 -- since if there is a performance degradation, it's on cards that simply don't work at all right now -- and if you're able to work on a followup patch that only performs the clock-lowering after the first error, I think that'd be a handy patch to have around. Does that sound good? Thanks! - Chris. -- Chris Ball c...@laptop.org http://printf.net/ One Laptop Per Child -- Manoj Iyer Ubuntu/Canonical Hardware Enablement
[PATCH v2 0/3] Make fault injection available for MMC IO
The first version of this patch is a part of mmc non-blocking v9 patchset: [PATCH v9 11/12] mmc: core: add random fault injection change log: v2 - Resolve build issue in mmc core.c due to multiple init_module by removing the fault inject module. - Export fault injection functions to make them available for modules - Update fault injection documentation on MMC IO Per Forlin (3): fault-inject: make fault injection available for modules mmc: core: add random fault injection fault injection: add documentation on MMC IO fault injection Documentation/fault-injection/fault-injection.txt |5 ++ drivers/mmc/core/core.c | 58 + drivers/mmc/core/debugfs.c|5 ++ include/linux/mmc/host.h |3 + lib/Kconfig.debug | 11 lib/fault-inject.c|2 + 6 files changed, 84 insertions(+), 0 deletions(-) -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/3] fault-inject: make fault injection available for modules
export symbols should_fail() and init_fault_attr_dentries() in order to make modules use the fault injection functionality Signed-off-by: Per Forlin per.for...@linaro.org --- lib/fault-inject.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/lib/fault-inject.c b/lib/fault-inject.c index 7e65af7..cd28364 100644 --- a/lib/fault-inject.c +++ b/lib/fault-inject.c @@ -131,6 +131,7 @@ bool should_fail(struct fault_attr *attr, ssize_t size) return true; } +EXPORT_SYMBOL(should_fail); #ifdef CONFIG_FAULT_INJECTION_DEBUG_FS @@ -311,5 +312,6 @@ fail: cleanup_fault_attr_dentries(attr); return -ENOMEM; } +EXPORT_SYMBOL(init_fault_attr_dentries); #endif /* CONFIG_FAULT_INJECTION_DEBUG_FS */ -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] fault injection: add documentation on MMC IO fault injection
Add description on how to enable random fault injection for MMC IO Signed-off-by: Per Forlin per.for...@linaro.org --- Documentation/fault-injection/fault-injection.txt |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/Documentation/fault-injection/fault-injection.txt b/Documentation/fault-injection/fault-injection.txt index 7be15e4..27eede4 100644 --- a/Documentation/fault-injection/fault-injection.txt +++ b/Documentation/fault-injection/fault-injection.txt @@ -21,6 +21,11 @@ o fail_make_request /sys/block/device/make-it-fail or /sys/block/device/partition/make-it-fail. (generic_make_request()) +o fail_mmc_request + + injects MMC data errors on devices permitted by setting + /sys/kernel/debug/mmc0/make-it-fail + Configure fault-injection capabilities behavior --- -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] mmc: core: add random fault injection
On 07/19/2011 02:31 PM, Per Forlin wrote: This adds support to inject data errors after a completed host transfer. The mmc core will return error even though the host transfer is successful. This simple fault injection proved to be very useful to test the non-blocking error handling in the mmc_blk_issue_rw_rq(). Random faults can also test how the host driver handles pre_req() and post_req() in case of errors. Signed-off-by: Per Forlinper.for...@linaro.org --- drivers/mmc/core/core.c| 58 drivers/mmc/core/debugfs.c |5 include/linux/mmc/host.h |3 ++ lib/Kconfig.debug | 11 4 files changed, 77 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index ab36c7b..3f822b4 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -23,6 +23,8 @@ #includelinux/log2.h #includelinux/regulator/consumer.h #includelinux/pm_runtime.h +#includelinux/fault-inject.h +#includelinux/random.h As a suggestion, would you want to also use '#ifdef CONFIG_FAIL_MMC_REQUEST' for fault-inject.h and random.h? If they are not used in a non-debug linux kernel configuration, could this possibly cause a little extra code bloat if they are a part of a production Linux kernel compile/configuration? #includelinux/mmc/card.h #includelinux/mmc/host.h @@ -82,6 +84,58 @@ static void mmc_flush_scheduled_work(void) flush_workqueue(workqueue); } +#ifdef CONFIG_FAIL_MMC_REQUEST + +static DECLARE_FAULT_ATTR(fail_mmc_request); + +static int __init setup_fail_mmc_request(char *str) Function comment header somewhere (here or the .h file?) +{ + return setup_fault_attr(fail_mmc_request, str); +} +__setup(fail_mmc_request=, setup_fail_mmc_request); + +static void mmc_should_fail_request(struct mmc_host *host, + struct mmc_request *mrq) +{ Function comment header somewhere (here or the .h file)? + struct mmc_command *cmd = mrq-cmd; + struct mmc_data *data = mrq-data; + static const int data_errors[] = { + -ETIMEDOUT, + -EILSEQ, + -EIO, + }; + + if (!data) + return; + Should 'if (!cmd)' also be checked or is this guaranteed to always have a valid value for this function (and if there are certain commands in 'struct mmc_command *cmd = mrq-cmd' that will not work with this function then that is something that should be documented in the function comment header)? + if (cmd-error || data-error || !host-make_it_fail || + !should_fail(fail_mmc_request, data-blksz * data-blocks)) + return; + + data-error = data_errors[random32() % ARRAY_SIZE(data_errors)]; + data-bytes_xfered = (random32() % (data-bytes_xfered 9)) 9; +} + +static int __init fail_mmc_request_debugfs(void) +{ + return init_fault_attr_dentries(fail_mmc_request, + fail_mmc_request); +} + +#else /* CONFIG_FAIL_MMC_REQUEST */ + +static void mmc_should_fail_request(struct mmc_host *host, + struct mmc_request *mrq) +{ +} + +static int __init fail_mmc_request_debugfs(void) +{ + return 0; +} +#endif /* CONFIG_FAIL_MMC_REQUEST */ + + /** *mmc_request_done - finish processing an MMC request *@host: MMC host which completed request @@ -108,6 +162,8 @@ void mmc_request_done(struct mmc_host *host, struct mmc_request *mrq) cmd-error = 0; host-ops-request(host, mrq); } else { + mmc_should_fail_request(host, mrq); + led_trigger_event(host-led, LED_OFF); pr_debug(%s: req done (CMD%u): %d: %08x %08x %08x %08x\n, @@ -2064,6 +2120,8 @@ static int __init mmc_init(void) if (ret) goto unregister_host_class; + fail_mmc_request_debugfs(); + return 0; unregister_host_class: diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c index 998797e..588e76f 100644 --- a/drivers/mmc/core/debugfs.c +++ b/drivers/mmc/core/debugfs.c @@ -188,6 +188,11 @@ void mmc_add_host_debugfs(struct mmc_host *host) root,host-clk_delay)) goto err_node; #endif +#ifdef CONFIG_FAIL_MMC_REQUEST + if (!debugfs_create_u8(make-it-fail, S_IRUSR | S_IWUSR, + root,host-make_it_fail)) + goto err_node; +#endif return; err_node: diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 771455f..250b46d 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -303,6 +303,9 @@ struct mmc_host { struct mmc_async_req*areq; /* active async req */ +#ifdef CONFIG_FAIL_MMC_REQUEST + u8 make_it_fail; +#endif unsigned long private[0] cacheline_aligned; }; diff