Re: [PATCH/RFC] mmc: tmio: Fix timeout value for command request
Hi,
@@ -230,7 +230,7 @@ static void tmio_mmc_reset_work(struct work_struct *work)
*/
if (IS_ERR_OR_NULL(mrq)
|| time_is_after_jiffies(host-last_req_ts +
- msecs_to_jiffies(2000))) {
+ msecs_to_jiffies(5000))) {
spin_unlock_irqrestore(host-lock, flags);
return;
}
@@ -818,7 +818,7 @@ static void tmio_mmc_request(struct mmc_host *mmc, struct
mmc_request *mrq)
ret = tmio_mmc_start_command(host, mrq-cmd);
if (!ret) {
schedule_delayed_work(host-delayed_reset_work,
- msecs_to_jiffies(2000));
+ msecs_to_jiffies(5000));
What about using a define here since the same kind of magic value is
used in two different places?
Kind regards,
Wolfram
signature.asc
Description: Digital signature
Re: [PATCH V2 2/3] i2c-piix4: Use Macro for AMD CZ SMBus device ID
On Thu, Jun 11, 2015 at 08:11:46PM +0800, Wan ZongShun wrote:
Change AMD CZ SMBUS device ID from 0x790b to
use Macro definition
Signed-off-by: Wan ZongShun vincent@amd.com
I think it makes sense that this patch goes in via MMC. This I2C change
is trivial, but for MMC there is more to handle. I don't expect
conflicts. So:
Acked-by: Wolfram Sang w...@the-dreams.de
---
drivers/i2c/busses/i2c-piix4.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index 67cbec6..630bce6 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -245,7 +245,7 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
PIIX4_dev-device == PCI_DEVICE_ID_AMD_HUDSON2_SMBUS
PIIX4_dev-revision = 0x41) ||
(PIIX4_dev-vendor == PCI_VENDOR_ID_AMD
- PIIX4_dev-device == 0x790b
+ PIIX4_dev-device == PCI_DEVICE_ID_AMD_KERNCZ_SMBUS
PIIX4_dev-revision = 0x49))
smb_en = 0x00;
else
@@ -545,7 +545,7 @@ static const struct pci_device_id piix4_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_IXP400_SMBUS) },
{ PCI_DEVICE(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_SBX00_SMBUS) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_HUDSON2_SMBUS) },
- { PCI_DEVICE(PCI_VENDOR_ID_AMD, 0x790b) },
+ { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_KERNCZ_SMBUS) },
{ PCI_DEVICE(PCI_VENDOR_ID_SERVERWORKS,
PCI_DEVICE_ID_SERVERWORKS_OSB4) },
{ PCI_DEVICE(PCI_VENDOR_ID_SERVERWORKS,
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
signature.asc
Description: Digital signature
[PATCH 5/7] mmc: host: sdhci-esdhc-imx: fix broken email address
My Pengutronix address is not valid anymore, redirect people to the Pengutronix
kernel team.
Reported-by: Harald Geyer har...@ccbib.org
Signed-off-by: Wolfram Sang w...@the-dreams.de
Acked-by: Robert Schwebel r.schwe...@pengutronix.de
---
These patches shall go via the individual subsystem trees.
drivers/mmc/host/sdhci-esdhc-imx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
b/drivers/mmc/host/sdhci-esdhc-imx.c
index 10ef8244a23963..3d7aa50e04578a 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -4,7 +4,7 @@
* derived from the OF-version.
*
* Copyright (c) 2010 Pengutronix e.K.
- * Author: Wolfram Sang w.s...@pengutronix.de
+ * Author: Wolfram Sang ker...@pengutronix.de
*
* 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
@@ -1173,5 +1173,5 @@ static struct platform_driver sdhci_esdhc_imx_driver = {
module_platform_driver(sdhci_esdhc_imx_driver);
MODULE_DESCRIPTION(SDHCI driver for Freescale i.MX eSDHC);
-MODULE_AUTHOR(Wolfram Sang w.s...@pengutronix.de);
+MODULE_AUTHOR(Wolfram Sang ker...@pengutronix.de);
MODULE_LICENSE(GPL v2);
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] mmc: host: don't use devm_pinctrl_get_select_default() in probe
Since commit ab78029ecc34 (drivers/pinctrl: grab default handles from device core), we can rely on device core for setting the default pins. Signed-off-by: Wolfram Sang w...@the-dreams.de --- drivers/mmc/host/mvsdio.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c index 4f8618f4522d..a2cb92851f1f 100644 --- a/drivers/mmc/host/mvsdio.c +++ b/drivers/mmc/host/mvsdio.c @@ -25,7 +25,6 @@ #include linux/of_irq.h #include linux/mmc/host.h #include linux/mmc/slot-gpio.h -#include linux/pinctrl/consumer.h #include asm/sizes.h #include asm/unaligned.h @@ -704,7 +703,6 @@ static int mvsd_probe(struct platform_device *pdev) const struct mbus_dram_target_info *dram; struct resource *r; int ret, irq; - struct pinctrl *pinctrl; r = platform_get_resource(pdev, IORESOURCE_MEM, 0); irq = platform_get_irq(pdev, 0); @@ -721,10 +719,6 @@ static int mvsd_probe(struct platform_device *pdev) host-mmc = mmc; host-dev = pdev-dev; - pinctrl = devm_pinctrl_get_select_default(pdev-dev); - if (IS_ERR(pinctrl)) - dev_warn(pdev-dev, no pins associated\n); - /* * Some non-DT platforms do not pass a clock, and the clock * frequency is passed through platform_data. On DT platforms, -- 2.1.3 -- 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 11/17] i2c: nomadik: Convert to devm functions
Would a way forward be to let you carry all the patches through your tree? I believe all but patch 17 can be safely merged. It is only this one that depends on the changes in the amba bus, so we can put this one on hold for a while. I'd favour this. Will do this next week unless somebody objects. signature.asc Description: Digital signature
Re: [PATCH V2 11/17] i2c: nomadik: Convert to devm functions
On Thu, Feb 13, 2014 at 03:09:02PM +0100, Ulf Hansson wrote:
Use devm_* functions to simplify code and error handling.
Cc: Alessandro Rubini rub...@unipv.it
Cc: Linus Walleij linus.wall...@linaro.org
Cc: Wolfram Sang w...@the-dreams.de
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
Changes in v2:
Rebased on top of latest i2c-nomadik branch.
Since this depends on Linus' patch already, I think it would be cleaner
if I pick this kinda unrelated (but wanted) devm patch, and ack the
PM stuff. If this for some reason makes things more complicated, I can
also simply ack this one.
- dev-virtbase = ioremap(adev-res.start, resource_size(adev-res));
+ dev-virtbase = devm_ioremap(adev-dev, adev-res.start,
+ resource_size(adev-res));
if (!dev-virtbase) {
ret = -ENOMEM;
IS_ERR()!
signature.asc
Description: Digital signature
Re: [PATCH] mmc: core: sd: implement proper support for sd3.0 au sizes
On Tue, Nov 26, 2013 at 02:16:25AM +0100, Wolfram Sang wrote: This reverts and updates commit 6fd0a4cc541b9a528eacc1d31ca47eb1ae7a (mmc: sd: fix the maximum au_size for SD3.0). The au_size for SD3.0 cannot be achieved by a simple bit shift, so this needs to be implemented differently. Also, don't print the warning in case of 0 since 'not defined' is different from 'invalid'. Signed-off-by: Wolfram Sang w...@the-dreams.de Cc: Jaehoon Chung jh80.ch...@samsung.com Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: H Hartley Sweeten hartl...@visionengravers.com Ping! Chris are you there? This got reviewed and acked, and I'd even think this could be suitable for stable. signature.asc Description: Digital signature
Re: [PATCH] mmc: core: sd: implement proper support for sd3.0 au sizes
On Mon, Jan 13, 2014 at 06:04:32PM +, Chris Ball wrote: Hi Wolfram, On Mon, Jan 13 2014, Wolfram Sang wrote: On Tue, Nov 26, 2013 at 02:16:25AM +0100, Wolfram Sang wrote: This reverts and updates commit 6fd0a4cc541b9a528eacc1d31ca47eb1ae7a (mmc: sd: fix the maximum au_size for SD3.0). The au_size for SD3.0 cannot be achieved by a simple bit shift, so this needs to be implemented differently. Also, don't print the warning in case of 0 since 'not defined' is different from 'invalid'. Ping! Chris are you there? This got reviewed and acked, and I'd even think this could be suitable for stable. Sorry about that! I agree, pushed to mmc-next for 3.14 with stable@ tag. Thanks. As buildbot reports, this will probably need another patch for blackfin :/ Sorry about that! From: Wolfram Sang w...@the-dreams.de Subject: [PATCH] mmc: core: add #include to support size defintions Fixes an error found by buildbot on blackfin after adding the fix for sd3.0 au sizes: drivers/mmc/core/sd.c:49:5: error: 'SZ_16K' undeclared here (not in a function) Signed-off-by: Wolfram Sang w...@the-dreams.de --- drivers/mmc/core/sd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index 6f42050..585d44d 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -11,6 +11,7 @@ */ #include linux/err.h +#include linux/sizes.h #include linux/slab.h #include linux/stat.h #include linux/pm_runtime.h signature.asc Description: Digital signature
Re: [PATCH] mmc: core: sd: implement proper support for sd3.0 au sizes
On Tue, Nov 26, 2013 at 02:16:25AM +0100, Wolfram Sang wrote: This reverts and updates commit 6fd0a4cc541b9a528eacc1d31ca47eb1ae7a (mmc: sd: fix the maximum au_size for SD3.0). The au_size for SD3.0 cannot be achieved by a simple bit shift, so this needs to be implemented differently. Also, don't print the warning in case of 0 since 'not defined' is different from 'invalid'. Signed-off-by: Wolfram Sang w...@the-dreams.de Cc: Jaehoon Chung jh80.ch...@samsung.com Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: H Hartley Sweeten hartl...@visionengravers.com Ping. signature.asc Description: Digital signature
Re: [PATCH] mmc: core: sd: implement proper support for sd3.0 au sizes
Good catch. It's right. Looks good to me. Thanks. Then this patch should probably go to stable as well... signature.asc Description: Digital signature
[PATCH] mmc: core: sd: implement proper support for sd3.0 au sizes
This reverts and updates commit 6fd0a4cc541b9a528eacc1d31ca47eb1ae7a
(mmc: sd: fix the maximum au_size for SD3.0). The au_size for SD3.0
cannot be achieved by a simple bit shift, so this needs to be
implemented differently. Also, don't print the warning in case of 0
since 'not defined' is different from 'invalid'.
Signed-off-by: Wolfram Sang w...@the-dreams.de
Cc: Jaehoon Chung jh80.ch...@samsung.com
Cc: Kyungmin Park kyungmin.p...@samsung.com
Cc: H Hartley Sweeten hartl...@visionengravers.com
---
Only tested with non SD3.0 cards (au = 0 and au = 9). Testers for 3.0 cards
much appreciated.
drivers/mmc/core/sd.c | 36 +---
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 6f42050..3b5ac4d 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -45,6 +45,13 @@ static const unsigned int tacc_mant[] = {
35, 40, 45, 50, 55, 60, 70, 80,
};
+static const unsigned int sd_au_size[] = {
+ 0, SZ_16K / 512, SZ_32K / 512, SZ_64K / 512,
+ SZ_128K / 512, SZ_256K / 512, SZ_512K / 512, SZ_1M / 512,
+ SZ_2M / 512,SZ_4M / 512,SZ_8M / 512,(SZ_8M + SZ_4M)
/ 512,
+ SZ_16M / 512, (SZ_16M + SZ_8M) / 512, SZ_32M / 512, SZ_64M / 512,
+};
+
#define UNSTUFF_BITS(resp,start,size) \
({ \
const int __size = size;\
@@ -216,7 +223,7 @@ static int mmc_decode_scr(struct mmc_card *card)
static int mmc_read_ssr(struct mmc_card *card)
{
unsigned int au, es, et, eo;
- int err, i, max_au;
+ int err, i;
u32 *ssr;
if (!(card-csd.cmdclass CCC_APP_SPEC)) {
@@ -240,26 +247,25 @@ static int mmc_read_ssr(struct mmc_card *card)
for (i = 0; i 16; i++)
ssr[i] = be32_to_cpu(ssr[i]);
- /* SD3.0 increases max AU size to 64MB (0xF) from 4MB (0x9) */
- max_au = card-scr.sda_spec3 ? 0xF : 0x9;
-
/*
* UNSTUFF_BITS only works with four u32s so we have to offset the
* bitfield positions accordingly.
*/
au = UNSTUFF_BITS(ssr, 428 - 384, 4);
- if (au 0 au = max_au) {
- card-ssr.au = 1 (au + 4);
- es = UNSTUFF_BITS(ssr, 408 - 384, 16);
- et = UNSTUFF_BITS(ssr, 402 - 384, 6);
- eo = UNSTUFF_BITS(ssr, 400 - 384, 2);
- if (es et) {
- card-ssr.erase_timeout = (et * 1000) / es;
- card-ssr.erase_offset = eo * 1000;
+ if (au) {
+ if (au = 9 || card-scr.sda_spec3) {
+ card-ssr.au = sd_au_size[au];
+ es = UNSTUFF_BITS(ssr, 408 - 384, 16);
+ et = UNSTUFF_BITS(ssr, 402 - 384, 6);
+ if (es et) {
+ eo = UNSTUFF_BITS(ssr, 400 - 384, 2);
+ card-ssr.erase_timeout = (et * 1000) / es;
+ card-ssr.erase_offset = eo * 1000;
+ }
+ } else {
+ pr_warning(%s: SD Status: Invalid Allocation Unit
size.\n,
+ mmc_hostname(card-host));
}
- } else {
- pr_warning(%s: SD Status: Invalid Allocation Unit
- size.\n, mmc_hostname(card-host));
}
out:
kfree(ssr);
--
1.8.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
[PATCH 0/4] remove devm_pinctrl_get_select_default from drivers
The core does this automatically, no need to do this explicitly in drivers. Those patchces should go via the respective subtrees. Thanks! Wolfram Sang (4): drivers/gpu/drm/tilcdc: don't use devm_pinctrl_get_select_default() in probe drivers/mmc/host: don't use devm_pinctrl_get_select_default() in probe drivers/pwm: don't use devm_pinctrl_get_select_default() in probe sound/soc/atmel: don't use devm_pinctrl_get_select_default() in probe drivers/gpu/drm/tilcdc/tilcdc_panel.c | 6 -- drivers/gpu/drm/tilcdc/tilcdc_slave.c | 6 -- drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 6 -- drivers/mmc/host/mvsdio.c | 7 +-- drivers/mmc/host/omap_hsmmc.c | 7 --- drivers/mmc/host/sdhci-esdhc-imx.c | 8 drivers/pwm/pwm-tiecap.c | 6 -- drivers/pwm/pwm-tiehrpwm.c | 6 -- sound/soc/atmel/atmel_wm8904.c | 8 9 files changed, 1 insertion(+), 59 deletions(-) -- 1.8.4.rc3 -- 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/4] drivers/mmc/host: don't use devm_pinctrl_get_select_default() in probe
Since commit ab78029 (drivers/pinctrl: grab default handles from device core),
we can rely on device core for setting the default pins. Compile tested only.
Acked-by: Linus Walleij linus.wall...@linaro.org (personally at LCE13)
Signed-off-by: Wolfram Sang w...@the-dreams.de
---
drivers/mmc/host/mvsdio.c | 7 +--
drivers/mmc/host/omap_hsmmc.c | 7 ---
drivers/mmc/host/sdhci-esdhc-imx.c | 8
3 files changed, 1 insertion(+), 21 deletions(-)
diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
index 06c5b0b..8c9f448 100644
--- a/drivers/mmc/host/mvsdio.c
+++ b/drivers/mmc/host/mvsdio.c
@@ -25,7 +25,6 @@
#include linux/of_irq.h
#include linux/mmc/host.h
#include linux/mmc/slot-gpio.h
-#include linux/pinctrl/consumer.h
#include asm/sizes.h
#include asm/unaligned.h
@@ -685,7 +684,7 @@ static int __init mvsd_probe(struct platform_device *pdev)
const struct mbus_dram_target_info *dram;
struct resource *r;
int ret, irq;
- struct pinctrl *pinctrl;
+ int gpio_card_detect, gpio_write_protect;
r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
irq = platform_get_irq(pdev, 0);
@@ -702,10 +701,6 @@ static int __init mvsd_probe(struct platform_device *pdev)
host-mmc = mmc;
host-dev = pdev-dev;
- pinctrl = devm_pinctrl_get_select_default(pdev-dev);
- if (IS_ERR(pinctrl))
- dev_warn(pdev-dev, no pins associated\n);
-
/*
* Some non-DT platforms do not pass a clock, and the clock
* frequency is passed through platform_data. On DT platforms,
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 6ac63df..f6606cf 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -38,7 +38,6 @@
#include linux/io.h
#include linux/gpio.h
#include linux/regulator/consumer.h
-#include linux/pinctrl/consumer.h
#include linux/pm_runtime.h
#include linux/platform_data/mmc-omap.h
@@ -1777,7 +1776,6 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
const struct of_device_id *match;
dma_cap_mask_t mask;
unsigned tx_req, rx_req;
- struct pinctrl *pinctrl;
match = of_match_device(of_match_ptr(omap_mmc_of_match), pdev-dev);
if (match) {
@@ -1996,11 +1994,6 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
omap_hsmmc_disable_irq(host);
- pinctrl = devm_pinctrl_get_select_default(pdev-dev);
- if (IS_ERR(pinctrl))
- dev_warn(pdev-dev,
- pins are not configured from the driver\n);
-
omap_hsmmc_protect_card(host);
mmc_add_host(mmc);
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
b/drivers/mmc/host/sdhci-esdhc-imx.c
index abc8cf0..0ac8b1a 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -25,7 +25,6 @@
#include linux/of.h
#include linux/of_device.h
#include linux/of_gpio.h
-#include linux/pinctrl/consumer.h
#include linux/platform_data/mmc-esdhc-imx.h
#include sdhci-pltfm.h
#include sdhci-esdhc.h
@@ -80,7 +79,6 @@ struct pltfm_imx_data {
int flags;
u32 scratchpad;
enum imx_esdhc_type devtype;
- struct pinctrl *pinctrl;
struct esdhc_platform_data boarddata;
struct clk *clk_ipg;
struct clk *clk_ahb;
@@ -568,12 +566,6 @@ static int sdhci_esdhc_imx_probe(struct platform_device
*pdev)
clk_prepare_enable(imx_data-clk_ipg);
clk_prepare_enable(imx_data-clk_ahb);
- imx_data-pinctrl = devm_pinctrl_get_select_default(pdev-dev);
- if (IS_ERR(imx_data-pinctrl)) {
- err = PTR_ERR(imx_data-pinctrl);
- goto disable_clk;
- }
-
host-quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
if (is_imx25_esdhc(imx_data) || is_imx35_esdhc(imx_data))
--
1.8.4.rc3
--
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] i2c: at91: convert to dma_request_slave_channel_compat()
On Mon, Apr 15, 2013 at 02:16:56PM +0200, ludovic.desroc...@atmel.com wrote: From: Ludovic Desroches ludovic.desroc...@atmel.com Use generic DMA DT helper. Platforms booting with or without DT populated are both supported. Signed-off-by: Ludovic Desroches ludovic.desroc...@atmel.com Applied to for-next, thanks! -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: sdhci / i.MX esdhc buswidth patches
On Mon, Sep 24, 2012 at 09:22:22AM +0200, Sascha Hauer wrote: The first patch is a generic sdhci driver cleanup change, the others are i.MX specific, adding 8bit bus support and fix version register readout. Sascha Sascha Hauer (3): mmc: sdhci: rename platform_8bit_width to platform_bus_width mmc: esdhc i.MX: Fix version register read mmc: esdhc i.MX: Support 8bit mode Whole series: Acked-by: Wolfram Sang w.s...@pengutronix.de -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 4/4] mmc: sdhci-esdhc-imx: Workaround for SD dat1 glitch causes system panic
On Thu, May 10, 2012 at 11:33:19AM +0200, Dirk Behme wrote: From: RichardZhu richard@linaro.org Some SD cards insertions will cause a glitch on SD dat1 which is also a card interrupt signal. Thus the wrongly generated card interrupt will make system panic because there's no registered sdio interrupt handler. This patch fixes this issue. Note: This is a workaround for i.MX6 version 1.0 silicon. It's fixed in hardware in version 1.1 silicon. Signed-off-by: Tony Lin tony@freescale.com Signed-off-by: RichardZhu richard@linaro.org --- drivers/mmc/host/sdhci-esdhc-imx.c | 18 +- 1 files changed, 17 insertions(+), 1 deletions(-) diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c index a13e75b..50a72a4 100644 --- a/drivers/mmc/host/sdhci-esdhc-imx.c +++ b/drivers/mmc/host/sdhci-esdhc-imx.c @@ -486,6 +486,7 @@ static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev) struct clk *clk; int err; struct pltfm_imx_data *imx_data; + u32 reg; host = sdhci_pltfm_init(pdev, sdhci_esdhc_imx_pdata); if (IS_ERR(host)) @@ -513,7 +514,22 @@ static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev) clk_prepare_enable(clk); pltfm_host-clk = clk; - host-quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL; + /* disable card interrupt enable bit, and clear status bit + * the default value of this enable bit is 1, but it should + * be 0 regarding to standard host controller spec 2.1.3. + * if this bit is 1, it may cause some problems. Very minor: the following problem? + * there's dat1 glitch when some cards inserting into the slot, + * thus wrongly generate a card interrupt that will cause + * system panic because it lacks of sdio handler + * following code will solve the problem. + */ + reg = sdhci_readl(host, SDHCI_INT_ENABLE); + reg = ~SDHCI_INT_CARD_INT; + sdhci_writel(host, reg, SDHCI_INT_ENABLE); + sdhci_writel(host, SDHCI_INT_CARD_INT, SDHCI_INT_STATUS); + + if (!is_imx25_esdhc(imx_data)) + host-quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL; NACK on the last two lines. Wrong rebase conflict resolution? Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] mmc: unbreak sdhci-esdhc-imx on i.MX25
On Wed, Apr 18, 2012 at 08:00:42PM -0400, Chris Ball wrote: Thanks, pushed to mmc-next for 3.4. If not done already, I think a stable-tag would make sense. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [MX25][MMC] mmc esdhc failure in 3.3
does that attached patch fix your problem ? YES, indeed it does to a certain degree. The timeout errors do not occur anymore and I can successfully perform many fs-ops, such as Interesting question is now why it worked on your older kernel? The code around BROKEN_TIMEOUT is there for much longer, I'd think. The speed of the card is excruciatingly slow, however. We roughly get between 34KB/s and 64KB/s from a dd if=/dev/zero write to any of the following partitions: Check the code below what is patched. That's currently the best solution for the errata. There might be better ones, though, but you'd need to find out. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [MX25][MMC] mmc esdhc failure in 3.3
Interesting question is now why it worked on your older kernel? The code around BROKEN_TIMEOUT is there for much longer, I'd think. not in fact it seems to have been broken from a long time and I think I know and you are saying the same, in fact :) Your patch came in around 2.6.37 and here it was said that 2.6.39 works fine. Might be random, though. because unlike the i.MX35 it seems that the i.MX25 manages to read properly the partition table even without the timeout quirk and it seems that I didn't do more extensive tests for this patch. Please do next time. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [MX25][MMC] mmc esdhc failure in 3.3
well I should have said enough extensive tests as I did a lot of tests on the 3 archs (i.MX25, i.MX35 and i.MX51). What I see here is that on i.MX35 and i.MX51 the problem is very easy to reproduce without extensive tests (card not detected) and that on i.MX25 it needs more tests as the card is properly detected. Ah, okay, thanks for the clarification! Well, since you retested now and found it is harder to trigger that could explain why nobody reported it and why 2.6.39 seems to work (although it maybe doesn't). Thanks for rechecking. If you submit the patch, you can add my Acked-by: Wolfram Sang w.s...@pengutronix.de Best regards. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [MX25][MMC] mmc esdhc failure in 3.3
Might be unrelated, however I have been keeping my eyes on the fix of ENGcm07207 quirk introduced with 16a790bcc. According to the IMX25CE.pdf, to abort data transfers on the AHB, software can reset the eSDHC by writing 1 to SYSCTL[24] (RSTA), which currently is not done with SDHCI_QUIRK_NO_MULTIBLOCK. It sets the max_blk_count to 1 instead of 65535. Not sure if this is also limiting the speed. Oh, it is limiting speed, for sure. SDHCI_QUIRK_NO_MULTIBLOCK is a _generic_ quirk which can be enabled by various buggy sdhci implementations, not only esdhc (which is pretty good at being buggy). It is not there to fix MX25 specific issues, that would have to be done seperately in sdhci-esdhc-imx.c. So, the above quirk is a big hammer solution. There might be more precise ones, but those still would have to be developed. I don't know of anyone planning to do this. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: iMX53 and MMC_CAP_SDIO_IRQ
Hi, The main problem is that the ancient Freescale 2.6.35 kernel is using the mx_sdhci driver, while in 3.3.0-rc6+ the generic sdhci driver is used, so it's not possible to simply compare the changes between the two versions. :-( Not via diff, but the structure of both drivers should still be somewhat similar. Probably this is a question for the Freescale people: why was the mx_sdhci necessary at all It probably was never necessary, it was just easier to hack on a forked driver, because you can't break other sdhci-users, I guess. Since large portions of the code are duplicated but issues have been fixed in a very, ahem, custom manner, this was never suitable for mainline. Back then, most vendors thought this is good enough. Luckily, times have changed a bit. and why is it not necessary any more? Because I wanted SD support in mainline, so I had to take a different path. Any help, hints and historical informations are highly appreciated, before I start to look deeper into this problem. They don't have a common ancestor. Just dig into both, you will recognize patterns, I guess. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: iMX53 and MMC_CAP_SDIO_IRQ
Ok, to sum this up: SDIO IRQs are working in mx_sdhci in the Freescale tree because it was properly implemented and tested, obviously. Not sure about properly, but, yes ;) SDIO IRQs are working for sdhci in mainline probably for other non-freescale platforms. Probably, but I can't say 'yes' for sure. You'd need to find out. You fixed the generic sdhci driver in mainline to work with iMX53 to get rid of the need for mx_sdhci, but probably never got the chance to test if SDIO IRQs are really working. I worked mainly with MX25/35 back then, and a little bit with MX51. IIRC I only had an SDIO card for which no Linux drivers were available, so I only could test that the card was detected :( That means, the subtle difference between mx_sdhci and sdhci to get SDIO IRQS to work on sdhci with iMX53 has yet to be found. Yup. Try 'git log --follow sdhci-esdhc-imx.c' to find people who worked with MX53 or SDIO using that driver. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [MX25][MMC] mmc esdhc failure in 3.3-rc5
What could have changed between 2.6.39.3 and 3.3-rc5 to trigger this behaviour? A quick look at git diff v2.6.39 drivers/mmc/host/sdhci-esdhc-imx.c I'd recommend: git log v2.6.39.. drivers/mmc/host/sdhci-esdhc-imx.c and look at those commits. You should also include the people who did those commits in CC when writing mails, otherwise they probably won't notice. Richard Zhu and Shawn Guo are from Freescale, and have more knowledge about the ESDHC cores. Wild guessing, 97e4ba6a5ea903a221d87bcabdaf01efb0a609a5 looks like the most likely candidate, but I may be totally wrong. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 01/10] MXS-DMA : move the mxs-dma.h to a more common place
On Thu, Jan 19, 2012 at 02:15:58PM +0800, Huang Shijie wrote: Move the header to a more common place. The DMA engine is not only used in mx23/mx28, but also used in mx50/mx6q. It will also be used in the future chips. Rename it to mxs-dma.h Signed-off-by: Huang Shijie b32...@freescale.com --- arch/arm/mach-mxs/include/mach/dma.h | 28 include/linux/mxs-dma.h | 28 2 files changed, 28 insertions(+), 28 deletions(-) delete mode 100644 arch/arm/mach-mxs/include/mach/dma.h create mode 100644 include/linux/mxs-dma.h Please use -M with git format-patch, so we can better see that it is a pure rename. I'd also suggest to squash patches up to 05/10 into this one, (collecting all the proper acks from maintainers) since the changes are trivial and it will reduce dependencies and temporary build-failures. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 00/10] patch set about the MXS-DMA
[2] patch 6 ~ patch 10: rewrite the last parameter of mxs_dma_prep_slave_sg(). For the sake of bitsec, at least patches #7 and #8 need to be one patch. That said, if I apply the series and then check out the commit at patch #7, you need to all mxs-dma client drivers, mxs-mmc, gpmi do not break. I'd say patch 6-10 should be squashed, simply. My personal preference is to change #9 to simply read the register when needed and do not expand the struct, but this is a minor thing. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 3/5 v3] ESDHC: Power management for ESDHC
On Mon, Jan 02, 2012 at 07:00:51PM -0500, Chris Ball wrote: Wolfram, do you have time to look at this? Looks like we need to expose suspend/resume hooks to -pltfm users for this use case -- I don't think any esdhc code should be in sdhci-pltfm.c. (I don't mind writing the patch if you agree that that's the correct solution here.) I won't have time to look at it, but what you say sounds reasonable to me. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 1/6] ESDHC: add PIO mode support
2) ... the aproach seems wrong to me. The quirks should be set depending on the compatible entry, e.g. if compatible == this_controller then quirks |= whatever_needed. Or? The quirk will not be set only depending on the compatible entry, the property entry can be, too. Where is that needed? I don't see how they are board-specific. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 1/6] ESDHC: add PIO mode support
For P2020E Rev1.0, the DMA can't work, we need the PIO mode. For P2020E, P1010E, MPC8536, we can't use the timeout value calculated by driver, we need the max value. For P1010E, the eSDHC controller can't use the max possible frequency(e.g. SDHC 50MHz), so we need one workaround to make the SD work This is SoC specific, not board specific, so you could check for fsl,p2020-esdhc for example. 1010 and 2020-rev1 would need proper compatibles as well. For eSDHC, after suspending, the power of esdhc controller will shutdown, we need to save the value of some registers before suspending, wich will used to restore the context after resuming. For eSDHC, the bit DCR[DMA__AHB2MAG_IRQ_BYPASS] can't be set automatically, so we need to set it manually If this is true for all revisions (be careful about the imx users), you don't need properties, but could simply set it. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 1/6] ESDHC: add PIO mode support
On Wed, Dec 14, 2011 at 10:19:37AM +0800, r66...@freescale.com wrote: From: Jerry Huang chang-ming.hu...@freescale.com For some FSL ESDHC controller(e.g. P2020E, Rev1.0), the SDHC can not work on DMA mode because of the hardware bug, so we set a broken dma flag and use PIO mode. Signed-off-by: Gao Guanhua b22...@freescale.com Signed-off-by: Jerry Huang chang-ming.hu...@freescale.com NACK for the series 1) If you introduce a new property you always have to document the binding which is missing. But you don't need to write it because... 2) ... the aproach seems wrong to me. The quirks should be set depending on the compatible entry, e.g. if compatible == this_controller then quirks |= whatever_needed. Or? Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [MXS MMC 1/5] Fix grammatical error in comment
On Tue, Dec 06, 2011 at 02:41:26PM +0100, Lothar Waßmann wrote: Signed-off-by: Lothar Waßmann l...@karo-electronics.de Acked-by: Wolfram Sang w.s...@pengutronix.de -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [MXS MMC 3/5] Add support for SSP/MMC ports 2 3
On Tue, Dec 06, 2011 at 02:41:28PM +0100, Lothar Waßmann wrote: i.MX28 has four SSP/MMC units, only two of which are currently usable. Signed-off-by: Lothar Waßmann l...@karo-electronics.de Reviewed-by: Wolfram Sang w.s...@pengutronix.de -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [MXS MMC 4/5] Check the return codes of clk_enable() and mxs_reset_block()
On Tue, Dec 06, 2011 at 02:41:29PM +0100, Lothar Waßmann wrote:
Add an int return value to mxs_mmc_reset(), so that the return code of
mxs_reset_block() can be promoted to the caller.
Also check the return code of clk_enable() in the probe function.
Signed-off-by: Lothar Waßmann l...@karo-electronics.de
- clk_enable(host-clk);
+ ret = clk_enable(host-clk);
+ if (ret) {
+ dev_err(mmc_dev(host-mmc),
+ %s: failed to enable clock: %d\n, __func__, ret);
+ goto out_clk_put;
+ }
- mxs_mmc_reset(host);
+ ret = mxs_mmc_reset(host);
+ if (ret) {
+ dev_err(mmc_dev(host-mmc),
+ %s: failed to reset controller: %d\n, __func__, ret);
+ goto out_clk_put;
+ }
Why __func__ here? dev_err and the msg itself should be indication enough?
Otherwise
Acked-by: Wolfram Sang w.s...@pengutronix.de
--
Pengutronix e.K. | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
Re: [MXS MMC 5/5] Add an appropriate MODULE_ALIAS
On Tue, Dec 06, 2011 at 02:41:30PM +0100, Lothar Waßmann wrote: Signed-off-by: Lothar Waßmann l...@karo-electronics.de Acked-by: Wolfram Sang w.s...@pengutronix.de -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] mmc: convert drivers/mmc/host/* to use module_platform_driver()
On Sat, Nov 26, 2011 at 04:19:53PM +0400, Anton Vorontsov wrote: On Sat, Nov 26, 2011 at 12:55:43PM +0800, Axel Lin wrote: This patch converts the drivers in drivers/mmc/host/* to use the module_platform_driver() macro which makes the code smaller and a bit simpler. Lost the original mail, but checked the patch using an archive. Acked-by: Wolfram Sang w.s...@pengutronix.de -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH]mmc: sdhci: use udelay instead of mdelay for some cases
Even there are some SDHCI hardwares cannot be stable in microseconds, I think this is also OK since they just need to wait for a few more loops. The total waiting time is the same as before. Well, I still think this is curing the symptoms not the cause. But will talk with Chris about it since we are both in Prague at the moment... -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH]mmc: sdhci: use udelay instead of mdelay for some cases
Hi, On Thu, Oct 27, 2011 at 12:18:53PM +0800, Shawn.Dong wrote: sdhci_set_clock or sdhci_reset or sdhci_send_command may be used in critical region which is protected by spin_lock_irqsave. Thus, these functions will delay the responsing of the kernel interrupts. Yes, so this needs to be improved, not the delay values. So in this case, using a mdelay will cause unnecessary latency. Our hardware, in most case will not cause 1ms to finish its job. Using udelay instead can reduce it. Could you guarantee this for all other SDHCI hardware out there as well? Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 2/2 v2] eSDHC: Fix errors when booting kernel with fsl esdhc
I would do it in the IO accessors. Thanks, Any update for your comment? It is still valid. You can go that road. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit
Hi, On Fri, Sep 09, 2011 at 08:05:46PM +0800, Roy Zang wrote: From: Xu lei b33...@freescale.com Freescale eSDHC registers only support 32-bit accesses, this patch ensures that all Freescale eSDHC register accesses are 32-bit. So, what about the byte-swapping that Anton needed? You are simply removing that if I am not mistaken. Signed-off-by: Xu lei b33...@freescale.com Signed-off-by: Roy Zang tie-fei.z...@freescale.com Signed-off-by: Kumar Gala ga...@kernel.crashing.org --- This is a patch resend http://patchwork.ozlabs.org/patch/106245/ based on latest Linus tree. Andrew, Could you help to pick up this patch first? mmc-list is not dead, it must go via Chris, I'd think. drivers/mmc/host/sdhci-of-esdhc.c | 18 ++ 1 files changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c index fe604df..40036f6 100644 --- a/drivers/mmc/host/sdhci-of-esdhc.c +++ b/drivers/mmc/host/sdhci-of-esdhc.c @@ -1,7 +1,7 @@ /* * Freescale eSDHC controller driver. * - * Copyright (c) 2007 Freescale Semiconductor, Inc. + * Copyright (c) 2007, 2010 Freescale Semiconductor, Inc. I wonder if such a small change has impact on the copyright. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit
Previously, I sent this patch together with another one. But technically they are separated patches. I got some comments from Anton about the patch. I will try to address the comment. but for this one, it is a stand along patch. I'd like to push it first. Okay. I do not see any comment about byte swapping about this patch. you may have mis-understanding here. I was misguided, yes, sorry. BE/LE issues with additional byteswapping are a mind twister :/ Having a second look, it looks okay to me. One could think about putting this into sdhci_platform as well, but this can be done later if needed. Sadly, I currently don't have hardware to test it. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 1/1] mmc: sdhci-esdhc: Change delay after setting clock from 100ms to 1ms
On Thu, Sep 01, 2011 at 01:51:15PM +0800, Tony Lin wrote: 1ms is enough for hardware to change the clock to stable. 100ms is too long. How do you know that? Can you be sure for PowerPC as well? Have you researched why the original author of the code has chosen that value? If so, please update your commit message with such infos. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] mxs-mmc: fix clock rate setting
About the patch itself: I didn't verify the formulas, but it does solve one special problem here. Thanks a lot! So: Does it solve any other special problem except for the wrong clock rate setting? Nope, but that it does :) -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] mxs-mmc: fix clock rate setting
On Mon, Jul 04, 2011 at 12:59:29PM +0200, Koen Beel wrote: On Mon, Jul 4, 2011 at 12:34 PM, Wolfram Sang w.s...@pengutronix.de wrote: Well, maybe not. My colleague complained and I think he is right that we are mapping div2 from the range 0 to 256 (inclusive!) to an 8-bit range. This must be wrong for one value per se. If you look at the context of the patch, you will find it's 'div2 - 1' than 'div2' gets written into register. Exactly. The '- 1' is why Koen changed the upper limit from 256 to = 256. The lower limit fix is currently 'if (div2 == 0) div2 == 1', which is a 2:1 mapping. Not good, or? So you are saying the patch is a right fix but not the most optimal one? In that case, it does not concern me. I acked it as an valid fix. The patches fixes a few things, but for div2, it is curing the symptoms, not the cause, I think. If you look at the formula in the datasheet: rate = ssp / (clock_divide * (1 + clock_rate)); In the code, the calculation of div2 is equal to '1 + clock_rate' (the 1 gets subtracted when the value is written to the register which is a bit unfortunate; doing it earlier would reduce confusion IMO). So that can never be 0, it is a divisor. We should really use DIV_ROUND_UP here. Even when not dealing with 0, this seems needed. Assume: ssp = 5760, wanted_rate = 2500, div1 = 2 will give div2 = int(1.152) = 1 (meaning 0 + 1, because div2 - 1 will be written) The rate will thus be: actual_rate = 5760 / 2 * (0 + 1) = 2880 - too fast! Or did I get something wrong? Regards, Wolfram Well, right now the clock freq. is set to the minimum clock value reaching the requested rate. Sorry, it gets a bit confusing: what do you mean with right now? In current implementation, the rate will be higher in lot's of cases (all cases where the requested clock freq. cannot exactly be made). Yes. But the thing is, the driver now enters dev_err, and returns without changing anything when the clock cannot be made as low as requested. In that case you will almost certainly end up with a clock being even higher then the lowest possible. So that not good I think. I might be better to set the clock as low as possible not matter what you to after that. Yes, might be a bit academic (who would want 4kHz ;)), but still correct. Shawn, do you agree? About the rounding. I don't think rounding is good. I think it would We do rounding already (int conversion). Just in the wrong direction. be better to choose between having at least the requested rate (as it is now; will result is somewhat higher clock then requested in many cases) You want a rate faster than what the card could do? , or having at maximum the requested clock rate. Both have there problems. If I understood you correctly, this is my DIV_ROUND_UP suggestion. Which problems does it have? Regards, Wolfram signature.asc Description: Digital signature
Re: [PATCH] mxs-mmc: fix clock rate setting
I think I misunderstood this suggestion previously. Using DIV_ROUND_UP would do the rounding in the correct direction. This should result in: the actual clock is as high as possible without being higher then the requested clock. Yes, that should be done in V2. And if you could also put the '- 1' to the line calculating div2, that would be an extra plus. I think it is a lot more readable to have the whole formula in one place, and not split up. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] mxs-mmc: fix clock rate setting
Well, maybe not. My colleague complained and I think he is right that we are mapping div2 from the range 0 to 256 (inclusive!) to an 8-bit range. This must be wrong for one value per se. If you look at the context of the patch, you will find it's 'div2 - 1' than 'div2' gets written into register. Exactly. The '- 1' is why Koen changed the upper limit from 256 to = 256. The lower limit fix is currently 'if (div2 == 0) div2 == 1', which is a 2:1 mapping. Not good, or? So you are saying the patch is a right fix but not the most optimal one? In that case, it does not concern me. I acked it as an valid fix. The patches fixes a few things, but for div2, it is curing the symptoms, not the cause, I think. If you look at the formula in the datasheet: rate = ssp / (clock_divide * (1 + clock_rate)); In the code, the calculation of div2 is equal to '1 + clock_rate' (the 1 gets subtracted when the value is written to the register which is a bit unfortunate; doing it earlier would reduce confusion IMO). So that can never be 0, it is a divisor. We should really use DIV_ROUND_UP here. Even when not dealing with 0, this seems needed. Assume: ssp = 5760, wanted_rate = 2500, div1 = 2 will give div2 = int(1.152) = 1 (meaning 0 + 1, because div2 - 1 will be written) The rate will thus be: actual_rate = 5760 / 2 * (0 + 1) = 2880 - too fast! Or did I get something wrong? Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 1/2] mmc: sdhci-esdhc: simplify cd_gpio, wp_gpio
On Fri, Jul 01, 2011 at 04:13:38PM -0700, Troy Kisky wrote: Currently, platform data is referenced by non init functions. Copy these fields into locally defined struct pltfm_imx_data. The code also currently frees imx_data on troubles with cd_gpio, but returns 0 anyway. Now the code ignores cd_gpio and wp_gpio if the values cause an error. Also, allow mx51/mx53 to use platform data. Signed-off-by: Troy Kisky troy.ki...@boundarydevices.com Have you seen this series from Shawn? [PATCH v4 0/4] Extend sdhci-esdhc-imx card_detect and write_protect support for mx5 I haven't checked if you are doing the same or similar, just that you are changing the same code. Please CC Shawn Guo and Richard Zhu if there is another patch series needed. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] mxs-mmc: fix clock rate setting
On Sun, Jul 03, 2011 at 05:28:52PM +0800, Shawn Guo wrote: On Sat, Jul 02, 2011 at 03:12:25PM +0200, Wolfram Sang wrote: On Fri, Jul 01, 2011 at 11:46:29PM +0800, Shawn Guo wrote: On Fri, Jul 01, 2011 at 04:44:09PM +0200, Wolfram Sang wrote: Hi, On Fri, Jul 01, 2011 at 03:26:39PM +0200, Koen Beel wrote: I send the patch as attachment for now. Fine with me in this case... But I'll have to look into another way of doing this. Corporate mail system is adding stupid disclaimers, gmail web ui is not working ok and corporate firewalls avoid using a different smtp server... Good luck with that! About the patch itself: I didn't verify the formulas, but it does solve one special problem here. Thanks a lot! So: Tested-by: Wolfram Sang w.s...@pengutronix.de @chris: If Shawn also likes the patch, I think this is a stable candidate. Thanks for the fixing, Koen. Acked-by: Shawn Guo shawn@freescale.com Well, maybe not. My colleague complained and I think he is right that we are mapping div2 from the range 0 to 256 (inclusive!) to an 8-bit range. This must be wrong for one value per se. If you look at the context of the patch, you will find it's 'div2 - 1' than 'div2' gets written into register. Exactly. The '- 1' is why Koen changed the upper limit from 256 to = 256. The lower limit fix is currently 'if (div2 == 0) div2 == 1', which is a 2:1 mapping. Not good, or? -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] mxs-mmc: fix clock rate setting
On Fri, Jul 01, 2011 at 11:46:29PM +0800, Shawn Guo wrote: On Fri, Jul 01, 2011 at 04:44:09PM +0200, Wolfram Sang wrote: Hi, On Fri, Jul 01, 2011 at 03:26:39PM +0200, Koen Beel wrote: I send the patch as attachment for now. Fine with me in this case... But I'll have to look into another way of doing this. Corporate mail system is adding stupid disclaimers, gmail web ui is not working ok and corporate firewalls avoid using a different smtp server... Good luck with that! About the patch itself: I didn't verify the formulas, but it does solve one special problem here. Thanks a lot! So: Tested-by: Wolfram Sang w.s...@pengutronix.de @chris: If Shawn also likes the patch, I think this is a stable candidate. Thanks for the fixing, Koen. Acked-by: Shawn Guo shawn@freescale.com Well, maybe not. My colleague complained and I think he is right that we are mapping div2 from the range 0 to 256 (inclusive!) to an 8-bit range. This must be wrong for one value per se. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] mxs-mmc: fix clock rate setting
Think the tabs problem was due to the gmail web ui. Hope it's better now. No 0xa0 anymore, but still no tabs. Oh, and lines are wrapped. Documentation/email-clients.txt says === Gmail (Web GUI) Does not work for sending patches. Gmail web client converts tabs to spaces automatically. At the same time it wraps lines every 78 chars with CRLF style line breaks although tab2space problem can be solved with external editor. Another problem is that Gmail will base64-encode any message that has a non-ASCII character. That includes things like European names. === I don't know if git-send-email can work directly with gmail, but it seems you need some kind of alternative approach. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] mxs-mmc: fix clock rate setting
Hi, On Fri, Jul 01, 2011 at 03:26:39PM +0200, Koen Beel wrote: I send the patch as attachment for now. Fine with me in this case... But I'll have to look into another way of doing this. Corporate mail system is adding stupid disclaimers, gmail web ui is not working ok and corporate firewalls avoid using a different smtp server... Good luck with that! About the patch itself: I didn't verify the formulas, but it does solve one special problem here. Thanks a lot! So: Tested-by: Wolfram Sang w.s...@pengutronix.de @chris: If Shawn also likes the patch, I think this is a stable candidate. Very minor nits: From ab610437fade5b30fdeacd9e959ddf95bc42587a Mon Sep 17 00:00:00 2001 From: Koen Beel koen.b...@barco.com Date: Thu, 30 Jun 2011 12:00:19 +0200 Subject: [PATCH] mxs-mmc: fix clock rate setting Fix clock rate setting on mxs-mmc driver. Previously, if div2 was zero the value for TIMING_CLOCK_RATE would have been 255 instead of 0. Also the limits for div1 (TIMING_CLOCK_DIVIDE) and div2 (TIMING_CLOCK_RATE + 1) where not correctly defined. Can easily be reproduced on mx23evk: default clock for high speed sdio cards is 50 MHz. With a SSP_CLK of 28.8 MHz (default), this resulted in an actual clock rate of about 56 kHz. Commit msg should linebreak at 72. Signed-off-by: Koen beel koen.b...@barco.com Capital 'B'? Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] mxs-mmc: fix clock rate setting
On Fri, Jul 01, 2011 at 08:08:48PM +0530, Sachin Nikam wrote: Can read_ahead_kb can be used to increase the number of read blocks? Unless you start a _new thread_ for your question and describe your problem in a bit more detail, you probably won't get an answer. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v3 2/4] mmc: sdhci-esdhc-imx: SDHCI_CARD_PRESENT does not get cleared
On Mon, Jun 20, 2011 at 06:38:43PM +0800, Shawn Guo wrote: The function esdhc_readl_le intends to clear bit SDHCI_CARD_PRESENT, when the card detect gpio tells there is no card. But it does not clear the bit actually. The patch gives a fix on that. Signed-off-by: Shawn Guo shawn@linaro.org For the third time ;) Acked-by: Wolfram Sang w.s...@pengutronix.de Should go to stable. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v3 2/4] mmc: sdhci-esdhc-imx: SDHCI_CARD_PRESENT does not get cleared
Should go to stable. I suppose that Chris will take care of it, That's correct. You can add the cc as Chris suggested, so the maintainer knows that you think it should go to stable. The final decission is up to the maintainer, though. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH V2] mmc: Enable the ADMA on esdhc imx driver
Hi Richard, It's strange that the sd card can't work on MX25 board without ADMA_BROKEN quirks. BTW how many cards/boards have you tested? Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH V2] mmc: Enable the ADMA on esdhc imx driver
Unfortunately register differences are common. Is there better approach than patching it in low level functions like this? I am all ears for suggestions... -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v2 2/4] mmc: sdhci-esdhc-imx: SDHCI_CARD_PRESENT does not get cleared
On Wed, Jun 15, 2011 at 07:15:20PM +0800, Shawn Guo wrote: The function esdhc_readl_le intends to clear bit SDHCI_CARD_PRESENT, when the card detect gpio tells there is no card. But it does not clear the bit actually. The patch gives a fix on that. Signed-off-by: Shawn Guo shawn@linaro.org Acked-by: Wolfram Sang w.s...@pengutronix.de Should go to stable... -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v2 4/4] mmc: sdhci-esdhc-imx: extend card_detect and write_protect support for mx5
On Wed, Jun 15, 2011 at 07:15:22PM +0800, Shawn Guo wrote: The patch extends card_detect and write_protect support to get mx5 family and more scenarios supported. The changes include: * Turn platform_data from optional to mandatory * Add cd_types and wp_types into platform_data to cover more use cases * Remove the use of flag ESDHC_FLAG_GPIO_FOR_CD * Adjust some machine codes to adopt the platform_data changes With this patch, card_detect and write_protect gets supported on mx5 based platforms. Signed-off-by: Shawn Guo shawn@linaro.org Looks like right direction to me, so far; some testers would be good, though. Acked-by: Wolfram Sang w.s...@pengutronix.de -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH V2] mmc: Enable the ADMA on esdhc imx driver
On Thu, Jun 16, 2011 at 02:06:29PM +0200, Pavel Machek wrote: On Thu 2011-06-16 14:00:09, Wolfram Sang wrote: Unfortunately register differences are common. Is there better approach than patching it in low level functions like this? I am all ears for suggestions... One way would be to move functions such as sdhci_activate_led() to the low level driver, There is no low level driver if the controller is done right. and introduce functions such as write_host_control() -- with no corresponding read_host_control, so that translation is easy to do... So, a seperate function for every register which differs from the standard? There are a lot of SoCs out there, and even more to come... Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v2 4/4] mmc: sdhci-esdhc-imx: extend card_detect and write_protect support for mx5
Sorry. I had some reason for moving esdhc_pltfm_get_ro around. We can not keep the existing approach (what I said above). For mx51 babbage example, esdhc1 uses internal WP while esdhc2 uses gpio WP. If we have esdhc_pltfm_get_ro handle gpio only and assign it to sdhci_esdhc_ops.get_ro in .probe only when wp_type is ESDHC_WP_GPIO, that works for esdhc2 but breaks esdhc1 WP function. So no, I will not change my code except adding NONE case handling there. Ok, fine with me. Thanks for checking! -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 2/4] mmc: sdhci-esdhc-imx: SDHCI_CARD_PRESENT does not get cleared
On Fri, Jun 10, 2011 at 06:42:50PM +0800, Shawn Guo wrote: The function esdhc_readl_le intends to clear bit SDHCI_CARD_PRESENT, when the card detect gpio tells there is no card. But it does not clear the bit actually. The patch gives a fix on that. Signed-off-by: Shawn Guo shawn@linaro.org Acked-by: Wolfram Sang w.s...@pengutronix.de Should go to stable, too. Thanks for catching it! -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 1/4] mmc: sdhci: fix interrupt storm from card detection
On Fri, Jun 10, 2011 at 06:42:49PM +0800, Shawn Guo wrote: The issue was initially found by Eric Benard as below. http://permalink.gmane.org/gmane.linux.ports.arm.kernel/108031 Not sure about other SDHCI based controller, but on Freescale eSDHC, the SDHCI_INT_CARD_INSERT bits will be immediately set again when it gets cleared, if a card is inserted. The driver need to mask the irq to prevent interrupt storm which will freeze the system. And the SDHCI_INT_CARD_REMOVE gets the same situation. The patch fixes the problem based on the initial idea from Eric Benard. Signed-off-by: Shawn Guo shawn@linaro.org Cc: Eric Benard e...@eukrea.com Hmm, that should get enough testing on non-imx (and even non-ARM) devices. And a comment describing the situation. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 3/4] mmc: sdhci-esdhc-imx: remove WP from flag ESDHC_FLAG_GPIO_FOR_CD_WP
On Fri, Jun 10, 2011 at 06:42:51PM +0800, Shawn Guo wrote: The use of flag ESDHC_FLAG_GPIO_FOR_CD_WP is all CD related. It does not necessarily need to bother WP in the flag name. Signed-off-by: Shawn Guo shawn@linaro.org I'd just squash it into the next one? -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v2 4/4] mmc: sdhci-esdhc-imx: extend card_detect and write_protect support for mx5
*/
- if (!cpu_is_mx25() !cpu_is_mx35())
- goto not_supported;
-
err = request_irq(gpio_to_irq(boarddata-cd_gpio), cd_irq,
IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
mmc_hostname(host-mmc), host);
@@ -307,10 +311,19 @@ static int __devinit sdhci_esdhc_imx_probe(struct
platform_device *pdev)
dev_warn(mmc_dev(host-mmc), request irq error\n);
goto no_card_detect_irq;
}
+ /* fall through */
- imx_data-flags |= ESDHC_FLAG_GPIO_FOR_CD;
- /* Now we have a working card_detect again */
+ case ESDHC_CD_SIGNAL:
+ /* we have a working card_detect back */
host-quirks = ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+ break;
+
+ case ESDHC_CD_PERMANENT:
+ host-mmc-caps = MMC_CAP_NONREMOVABLE;
+ break;
+
+ case ESDHC_CD_NONE:
+ break;
}
err = sdhci_add_host(host);
@@ -319,16 +332,20 @@ static int __devinit sdhci_esdhc_imx_probe(struct
platform_device *pdev)
return 0;
- no_card_detect_irq:
- gpio_free(boarddata-cd_gpio);
- no_card_detect_pin:
- boarddata-cd_gpio = err;
- not_supported:
- kfree(imx_data);
- err_add_host:
+err_add_host:
+ if (gpio_is_valid(boarddata-cd_gpio))
+ free_irq(gpio_to_irq(boarddata-cd_gpio), host);
+no_card_detect_irq:
+ if (gpio_is_valid(boarddata-cd_gpio))
+ gpio_free(boarddata-cd_gpio);
+ if (gpio_is_valid(boarddata-wp_gpio))
+ gpio_free(boarddata-wp_gpio);
+no_card_detect_pin:
+no_board_data:
clk_disable(pltfm_host-clk);
clk_put(pltfm_host-clk);
- err_clk_get:
+err_clk_get:
+ kfree(imx_data);
sdhci_pltfm_free(pdev);
return err;
}
@@ -343,14 +360,12 @@ static int __devexit sdhci_esdhc_imx_remove(struct
platform_device *pdev)
sdhci_remove_host(host, dead);
- if (boarddata gpio_is_valid(boarddata-wp_gpio))
+ if (gpio_is_valid(boarddata-wp_gpio))
gpio_free(boarddata-wp_gpio);
- if (boarddata gpio_is_valid(boarddata-cd_gpio)) {
+ if (gpio_is_valid(boarddata-cd_gpio)) {
+ free_irq(gpio_to_irq(boarddata-cd_gpio), host);
gpio_free(boarddata-cd_gpio);
-
- if (!(host-quirks SDHCI_QUIRK_BROKEN_CARD_DETECTION))
- free_irq(gpio_to_irq(boarddata-cd_gpio), host);
}
clk_disable(pltfm_host-clk);
--
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
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
Re: [PATCH 1/4] mmc: sdhci: fix interrupt storm from card detection
The issue was initially found by Eric Benard as below. http://permalink.gmane.org/gmane.linux.ports.arm.kernel/108031 Not sure about other SDHCI based controller, but on Freescale eSDHC, the SDHCI_INT_CARD_INSERT bits will be immediately set again when it gets cleared, if a card is inserted. The driver need to mask the irq to prevent interrupt storm which will freeze the system. And the SDHCI_INT_CARD_REMOVE gets the same situation. The patch fixes the problem based on the initial idea from Eric Benard. Signed-off-by: Shawn Guo shawn@linaro.org Cc: Eric Benard e...@eukrea.com Hmm, that should get enough testing on non-imx (and even non-ARM) devices. And a comment describing the situation. Agreed. Will add something in commit message to mention the situation. That's why I hope we can get the patch on mmc-next at I actually meant a comment in the code, so it will be obvious for later hackers why the extra steps are in there... -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [RFC] mmc: Enable the ADMA on esdhc imx driver
On Thu, Jun 02, 2011 at 05:12:10PM +0800, Richard Zhu wrote:
Eanble the ADMA mode on freescale esdhc imx driver,
tested on MX51 and MX53.
Please describe a little bit why the patch helps. Does it also work on mx25/35?
What does the new bit cover what the old one didn't cover? Why does the new one
even exist?
Signed-off-by: Richard Zhu richard@linaro.org
---
drivers/mmc/host/sdhci-esdhc-imx.c | 40 ++-
1 files changed, 34 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
b/drivers/mmc/host/sdhci-esdhc-imx.c
index a19967d..64f33cb 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -31,6 +31,8 @@
#define SDHCI_VENDOR_SPEC0xC0
#define SDHCI_VENDOR_SPEC_SDIO_QUIRK0x0002
+#define SDHCI_VENDOR_SPEC_INT_ADMA_ERR 0x1000
+
From the code below, this name is wrong because the bit is not in VENDOR_SPEC.
@@ -166,12 +189,16 @@ static void esdhc_writeb_le(struct sdhci_host *host, u8
val, int reg)
case SDHCI_HOST_CONTROL:
/* FSL messed up here, so we can just keep those two */
new_val = val (SDHCI_CTRL_LED | SDHCI_CTRL_4BITBUS);
- /* ensure the endianess */
+ /* ensure the endi1ness */
Ehrm?
- /* DMA mode bits are shifted */
- new_val |= (val SDHCI_CTRL_DMA_MASK) 5;
+ if (val SDHCI_CTRL_DMA_MASK) {
+ /* DMA mode bits are shifted */
+ new_val |= (val SDHCI_CTRL_DMA_MASK) 5;
+
+ esdhc_clrset_le(host, 0x, new_val, reg);
+ } else
+ esdhc_clrset_le(host, 0xff, val, reg);
Why can't we always write 16-bit? (and else-block needs braces)
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
Re: [PATCH 1/2] mmc: sdhci: Replace SDHCI_QUIRK_FORCE_BLK_SZ_2048 with a platform hook.
Hi Nicolas, :( I still like the io-accessor-method a lot better. That's okay -- nothing's final yet, I just wanted to get things moving again since we're out of quirk space now. I'm still happy to take a patch from you instead if we decide it's the better way to go. What about letting the platform specific code override the caps bits instead? That would work for many other things as well, like the various DMA quirks, etc. And that requires only one callback instead of one per parameter you might want to override. This should be made explicit with a dedicated callback of course, not via some magic in sdhci_readl() please. I proposed a kind-of fixup() function once, but someone (Olof?) didn't like it because it broke abstraction. However, after Shawn's patch series turning sdhci into a library, things might have become easier for this approach. Hmmm, I'd like to try it, just have to see when. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 1/2] mmc: sdhci: Replace SDHCI_QUIRK_FORCE_BLK_SZ_2048 with a pklatform hook.
On Sat, May 28, 2011 at 10:27:41PM -0400, Chris Ball wrote: Hi, On Sun, Feb 06 2011, Chris Ball wrote: Part of a quirk cleanup run. This quirk was only used by sdhci-esdhc. This patch is untested. Signed-off-by: Chris Ball c...@laptop.org Cc: Anton Vorontsov cbouatmai...@gmail.com Cc: Wolfram Sang w.s...@pengutronix.de I've queued this patch now, since we're out of quirk bits and didn't come up with any alternative patches. (Feel free to improve on it later.) :( I still like the io-accessor-method a lot better. I would have created a patch if I had got some feedback on my suggestion[1] (and will still do). Regards, Wolfram [1] http://thread.gmane.org/gmane.linux.kernel.mmc/5742/focus=5768 -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v3 0/4] Consolidate sdhci pltfm OF drivers and get them self registered
Any chance we can get this series into $NEXT_KERNEL? From the looks, I think it still has problems being a module. Will try to test it today. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v2 0/7] Consolidate sdhci pltfm OF drivers and get them self registered
Hi Shawn, Changes since v1: * Rebase on cjb's mmc-next tree Is it maybe possible that you get access to http://opensource.freescale.com/git or another machine? A branch to pull from would be more convenient, because the series does not apply to mmc-next anymore, so an extra step to go back in time is needed. (minor) When applying I got: Applying: mmc: sdhci: make sdhci-pltfm device drivers self registered /home/wsa/Kernel/linux-2.6/.git/rebase-apply/patch:384: trailing whitespace. /home/wsa/Kernel/linux-2.6/.git/rebase-apply/patch:817: space before tab in indent. struct tegra_sdhci_platform_data *plat; /home/wsa/Kernel/linux-2.6/.git/rebase-apply/patch:867: trailing whitespace. Applying: sdhci: rename sdhci-esdhc-imx.c to sdhci-esdhc.c /home/wsa/Kernel/linux-2.6/.git/rebase-apply/patch:780: trailing whitespace. See later comments for further issues. Thanks, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v2 0/7] Consolidate sdhci pltfm OF drivers and get them self registered
Hi Shawn, Should I go for v3 right now to address the patch applying problems and that ESDHC_IMX build issue, or hold for a while to see if you have more comments on v2? Please wait a little bit more. And what is your position on patch #5 which merges esdhc imx and mpc support into one? As Anton has voted a NO there, I would probably drop the patch if there is another person has strong opinion to get imx and mpc stay separated. This was the other main issue I spotted so far. I wanted to have another look tomorrow, yet the tendency is that I agree with Anton. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: hard wired wifi chip on sdio
I have a Freescale iMX25 based (tx25) board with a Marvell 8688 wireless module on sdio bus. My kernel is a rel_imx_2.6.35_11.03.00 from Freescale git repository. Please either a) use a _recent vanilla_ kernel or b) ask Freescale about their kernel directly. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: SD Patches
On Wed, Apr 27, 2011 at 07:13:22PM -0700, Subhash Jadavani wrote: I have also reviewed Arindam's SD3.0 patches and looks good to me. Although I haven't tested those yet. Thanks. To make it easier next time: The usual pattern for this is replying directly to the series to [PATCH 0/x] (if you mean the whole series) or reply to each [PATCH n/x] with something like Reviewed-by: Your name your@address Tested-by: Your name your@address (I have keyboard macros for those ;)) Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] mmc:sdhci:add support to request/free pins for controllers sharing hardware bus
* Where is the patch that implements the get_shared_pins() hook in your driver? We send the common level patch ahead to get the upstream agreement so that we can maintain our bottom level sdhci codes better. We will send the special driver patch after some time. Hmm, why should be there a functionality if there is no user for it? A reference user helps people understand how it is intended to be used. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v3 00/12] add support for host controller v3.00
On Tue, Apr 26, 2011 at 08:50:31AM -0400, Chris Ball wrote: Hi Arindam, On Tue, Apr 26 2011, Nath, Arindam wrote: Just a follow up on what you said. Is there any plan to merge the patches with your tree? Yes, sounds good -- linux-mmc@ folks, would anyone like to give a Tested- or Reviewed-by: on these patches? Considering this is a rather large changeset (thanks for doing it!), what about putting it into an extra -next round (i.e. for-2.6.41) unless somebody reviews or tests the series? I sadly can't, no hardware. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 0/5] consolidate sdhci pltfm OF drivers and get them self registered
Hi Shawn, 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. I stand corrected. I assumed the STM platforms did hit mainline meanwhile, but they did not. There really doesn't seem to be one user of sdhci-pltfm without using workarounds. (will make sdhc v3 make it better or worse? I get fears...) So, everything OK with your series. 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. sdhci-dove needs to overload readw/readl. If there is a user not needings such, i.e. only plain quirks (or even nothing, what a dream!), then a generic driver might be worthwhile. Can wait until we see such a user, though. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
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 0/5] consolidate sdhci pltfm OF drivers and get them self registered
Hi Shawn, On Fri, Mar 25, 2011 at 04:48:46PM +0800, Shawn Guo wrote: Here are what the patch set does. * Remove .probe and .remove hooks from sdhci-pltfm.c and make it be a pure common helper function providers. * Add .probe and .remove hooks for sdhci pltfm drivers sdhci-cns3xxx, sdhci-dove, sdhci-tegra, and sdhci-esdhc-imx to make them self registered with calling helper functions created above. * Migrate the use of sdhci_of_host and sdhci_of_data to sdhci_pltfm_host and sdhci_pltfm_data, so that OF version host and data structure works can be saved, and pltfm version works for both cases. * Add OF common helper stuff into sdhci-pltfm.c, and make OF version sdhci drivers sdhci-of-esdhc and sdhci-of-hlwd become self registered as well, so that sdhci-of-core.c and sdhci-of.h can be removed. * Consolidate the OF and pltfm esdhc drivers into one with sharing the same pair of .probe and .remove hooks. As a result, sdhci-esdhc-imx.c and sdhci-of-esdhc.c go away, while sdhci-esdhc.c comes in and works for both MPCxxx and i.MX. * Eliminate include/linux/mmc/sdhci-pltfm.h with moving stuff into drivers/mmc/host/sdhci-pltfm.h. And the benefits we gain from the changes are: * Get the sdhci device driver follow the Linux trend that driver makes the registration by its own. * sdhci-pltfm.c becomes simple and clean as it only has common helper stuff there now. * All sdhci device specific things are going back its own driver. * The dt and non-dt drivers are consolidated to use the same pair of .probe and .remove hooks. * SDHCI driver for Freescale eSDHC controller found on both MPCxxx and i.MX platforms is consolidated to use the same one .probe function. The patch set works against the tree below, and was only tested on i.mx51 babbage board, all other targets were build tested. git://git.secretlab.ca/git/linux-2.6.git devicetree/test Comments are welcomed and appreciated. First of all, thanks _a lot_ for doing this! Many people have promised to do such an approach, but you finally made it! Sorry for the long delay in reviewing it, I didn't want to rush a review so I needed some time for it which I didn't have much in the last weeks. Let's hope my battery will have enough power on my flight back to Germany ;) So, this is for now a purely visual review. I hope I can check V2 on real hardware then. 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. 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. 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. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 1/5] mmc: sdhci: make sdhci-pltfm device drivers self registered
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'.
+MODULE_LICENSE(GPL v2);
Just to be sure: Did you double-check if the original licenses were v2
or v2+?
--- 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?
+ return NULL;
}
I didn't pay much attention to the OF version of the tegra driver, since
it still is not upstream yet, right?
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
Re: [PATCH 3/5] mmc: sdhci: make sdhci-of device drivers self registered
+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?
[...]
+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.
--
Pengutronix e.K. | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
Re: [PATCH 4/5] mmc: sdhci: consolidate sdhci-of-esdhc and sdhci-esdhc-imx
config MMC_SDHCI_ESDHC_IMX
- bool SDHCI platform support for the Freescale eSDHC i.MX controller
+ bool SDHCI support for the Freescale eSDHC i.MX controller
depends on ARCH_MX25 || ARCH_MX35 || ARCH_MX5
depends on MMC_SDHCI
- select MMC_SDHCI_PLTFM
+ select MMC_SDHCI_ESDHC
select MMC_SDHCI_IO_ACCESSORS
help
- This selects the Freescale eSDHC controller support on the platform
- bus, found on platforms like mx35/51.
+ This selects the Freescale eSDHC controller support on platforms
+ like mx35/51.
While we are at it, mx25 could be added and mx53 (and you know better
what else will be coming ;))
diff --git a/drivers/mmc/host/sdhci-esdhc.c b/drivers/mmc/host/sdhci-esdhc.c
new file mode 100644
index 000..b3d1bc1
--- /dev/null
+++ b/drivers/mmc/host/sdhci-esdhc.c
@@ -0,0 +1,413 @@
+/*
+ * Freescale eSDHC controller driver for MPCxxx and i.MX.
+ *
+ * Copyright (c) 2007 Freescale Semiconductor, Inc.
+ * Author: Xiaobo Xie x@freescale.com
+ *
+ * Copyright (c) 2009 MontaVista Software, Inc.
+ * Author: Anton Vorontsov avoront...@ru.mvista.com
+ *
+ * Copyright (c) 2010 Pengutronix e.K.
+ * Author: Wolfram Sang w.s...@pengutronix.de
+ *
+ * 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.
+ */
+
+#include linux/io.h
+#include linux/delay.h
+#include linux/err.h
+#include linux/clk.h
+#include linux/mmc/host.h
+#include linux/mmc/sdhci-pltfm.h
+#ifdef CONFIG_MMC_SDHCI_ESDHC_IMX
+#include mach/hardware.h
+#endif
+#include sdhci.h
+#include sdhci-pltfm.h
+
+/*
+ * Ops and quirks for the Freescale eSDHC controller.
+ */
+
+#define ESDHC_DEFAULT_QUIRKS (SDHCI_QUIRK_FORCE_BLK_SZ_2048 | \
+ SDHCI_QUIRK_BROKEN_CARD_DETECTION | \
+ SDHCI_QUIRK_NO_BUSY_IRQ | \
+ SDHCI_QUIRK_NONSTANDARD_CLOCK | \
+ SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | \
+ SDHCI_QUIRK_PIO_NEEDS_DELAY | \
+ SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET | \
+ SDHCI_QUIRK_NO_CARD_NO_RESET)
These are not the current quirks. Meanwhile BROKEN_CARD_DETECTION is
gone as well as NO_CARD_NO_RESET (at least for imx). My additions to use
GPIOs for card detect and write protect are also not in this version.
[...]
+static struct sdhci_pltfm_data sdhci_esdhc_mpc_pdata = {
+ .quirks = ESDHC_DEFAULT_QUIRKS,
+ .ops = sdhci_esdhc_mpc_ops,
+};
+#endif
Please mark the #endif with comments of the expression they are
depending on, e.g.
#endif /* CONFIG_MMC_SDHCI_ESDHC_MPC */
if it is not immediately visible. That helps readability.
[...]
Phew, due to all these hardware bugs, the driver will get messy, even if
we try hard. Any chances that future revisions of the core will be
updated? :)
--
Pengutronix e.K. | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
Re: [PATCH 5/5] mmc: sdhci: merge two sdhci-pltfm.h into one
On Fri, Mar 25, 2011 at 04:48:51PM +0800, Shawn Guo wrote: The structure sdhci_pltfm_data is not necessarily to be in a public header like include/linux/mmc/sdhci-pltfm.h, so the patch moves it into drivers/mmc/host/sdhci-pltfm.h and eliminates the former one. Signed-off-by: Shawn Guo shawn@linaro.org Reviewed-by: Wolfram Sang w.s...@pengutronix.de -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 0/5] consolidate sdhci pltfm OF drivers and get them self registered
BTW, Are there reason to omit the sdhci-s3c.c? Maybe Mr. Jung will handle it. The difference is that sdhci-s3c.c was more like a fork of sdhci-pltfm while the others were users of it. With the new interface, s3c can be converted using pltfm more easily. If somebody is willing to do that, this is very much welcome. I'd suggest waiting for Shawn's interface to stabilize, though. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] mmc: USB SDIO/SD/MMC Host Controller (VUB300) driver Re-Re-Resubmission
Hi Tony, Add a driver for Elan Digital System's VUB300 chip which is a USB connected SDIO/SDmem/MMC host controller. A VUB300 chip enables a USB 2.0 or USB 1.1 connected host computer to use SDIO/SD/MMC cards without the need for a directly connected, for example via PCI, SDIO host controller. Signed-off-by: Anthony F Olech tony.ol...@elandigitalsystems.com --- This is the forth submission attempt. Looks to me that the concerns have not been addressed since last submission. I had a few a while ago, but Arnd added to it and summed it up nicely: http://ns3.spinics.net/lists/linux-mmc/msg06693.html There are no errors reported by scripts/checkpatch.pl Just to make sure: This only means that there are no obvious (= detectable by a script) flaws. That makes it ready for submission, not necessarily for inclusion. So, just resending will not be enough. This driver has been tested on a) 32bit x86 b) 64bit x86 c) dual processor d) PowerPC The test coverage is great, yet the driver needs to comply to the kernel coding rules. I'd suggest to look at Arnd's review and fix the issues. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v4] mmc: sdhci: add error checking for mmc_add_host
You means that didn't run start_host() before add_host failed. I think that need not to run stop_host. right? Right, and other stuff like registering the pm_notifier. To use mmc_free_host, better than mmc_remove_host. That would be true for most drivers. However, sdhci has encapsulated this in its own sdhci_free_host. So, you don't need to call this as well. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] mmc: sdhci-pci: Fix checkpatch.pl errors
On Mon, Apr 11, 2011 at 12:47:02PM +0300, Ameya Palande wrote: Hi Wolfram, On Fri, Apr 8, 2011 at 9:55 PM, Wolfram Sang w.s...@pengutronix.de wrote: Hi Ameya, On Tue, Apr 05, 2011 at 09:13:13PM +0300, Ameya Palande wrote: This patch fixes 21 errors and 6 warnings reported by checkpatch.pl Signed-off-by: Ameya Palande 2am...@gmail.com Looks okay to me. Have you checked that the binary does not differ? Thanks for your review! Binaries are same. Reviewed-by: Wolfram Sang w.s...@pengutronix.de -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [RFC] mmc: sdhci: work around broken dma boundary behavior
In my opinion this is more risky than the original patch because this will affect the behavior on all controllers that use sdhci, and not just on those ones that don't update SDHCI_DMA_ADDRESS themselves. Comments welcome! Any thoughts on which of the two approaches you prefer? Thanks for the ping. I prefer the latter approach but it should be given enough time testing in -next. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v4] mmc: sdhci: add error checking for mmc_add_host
On Mon, Apr 11, 2011 at 11:11:08AM +0900, Jaehoon Chung wrote: Sometimes we can't add the device,but we didn't check any error status. Need to check error status for mmc_add_host. And Missing regulator disable/put. Fixed them. Is there any special reason you don't want to answer the open question from the last two reviews? [PATCH v4] : merged for v3 [PATCH 1/2] and [PATCH 2/2] reviewed on Wolfram Sang This should go below the dashed line (---) Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
[PATCH] mmc: host: fix memory leak in mmc_add_host
led_trigger_register_simple() allocates memory which must not be leaked in the error-path of mmc_add_host. Move it past the only error-check in the function. Signed-off-by: Wolfram Sang w.s...@pengutronix.de --- I don't see any reason why it needs to be called before device_add. drivers/mmc/core/host.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c index 461e6a1..b29d3e8 100644 --- a/drivers/mmc/core/host.c +++ b/drivers/mmc/core/host.c @@ -325,12 +325,12 @@ int mmc_add_host(struct mmc_host *host) WARN_ON((host-caps MMC_CAP_SDIO_IRQ) !host-ops-enable_sdio_irq); - led_trigger_register_simple(dev_name(host-class_dev), host-led); - err = device_add(host-class_dev); if (err) return err; + led_trigger_register_simple(dev_name(host-class_dev), host-led); + #ifdef CONFIG_DEBUG_FS mmc_add_host_debugfs(host); #endif -- 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 v4] mmc: sdhci: add error checking for mmc_add_host
I just think good that remove the host...(any crash didn't occur). there is not special reason. Look at the mmc-core code, please (host.c). Compare what remove_host does and what add_host does before it fails. What do you think? Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v3 2/2] mmc: sdhci: add regulator disable/put when control error
Looks OK but could be merged into the previous patch. Thanks. :) yes, i can be merged into the previous patch. But i think good that separeted. anyway..don't mind merged or not? I prefer it merged, because it then fixes all forgotten cleanups in the error-path in one go. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v3 1/2] mmc: sdhci: add error checking for mmc_add_host
Hi,
On Tue, Apr 05, 2011 at 05:02:19PM +0900, Jaehoon Chung wrote:
Sometimes we can't add the device,but we didn't check any error status.
Need to check error status for mmc_add_host.
Signed-off-by: Jaehoon Chung jh80.ch...@samsung.com
Signed-off-by: Kyungin Park kyungmin.p...@samsung.com
---
drivers/mmc/host/sdhci.c | 20 ++--
1 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 9e15f41..1768ffb 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2025,7 +2025,9 @@ int sdhci_add_host(struct sdhci_host *host)
mmiowb();
- mmc_add_host(mmc);
+ ret = mmc_add_host(mmc);
+ if (unlikely(ret))
+ goto err_free_mmc;
printk(KERN_INFO %s: SDHCI controller on %s [%s] using %s\n,
mmc_hostname(mmc), host-hw_name, dev_name(mmc_dev(mmc)),
@@ -2036,15 +2038,29 @@ int sdhci_add_host(struct sdhci_host *host)
return 0;
+err_free_mmc:
+ mmc_remove_host(host);
As I asked in my last review: Why do you remove the host when add_host failed?
Doesn't that crash?
+
#ifdef SDHCI_USE_LEDS_CLASS
+ led_classdev_ungregister(host-led);
reset:
+#endif
sdhci_reset(host, SDHCI_RESET_ALL);
free_irq(host-irq, host);
-#endif
+ del_timer_sync(host-timer);
+
untasklet:
tasklet_kill(host-card_tasklet);
tasklet_kill(host-finish_tasklet);
+ if (host-flags SDHCI_USE_ADMA) {
+ kfree(host-adma_desc);
+ kfree(host-align_buffer);
+
+ host-adma_desc = NULL;
+ host-align_buffer = NULL;
+ }
The if-condition is not needed, kfree is NULL-safe.
+
return ret;
}
--
Pengutronix e.K. | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
Re: [PATCH] mmc: sdhci: fixed regulator control when suspend/resume
On Thu, Apr 07, 2011 at 01:23:52PM +0900, Jaehoon Chung wrote: Suspend/resume is working always enable regulator after resuming. (if there is host-vmmc) The regulator is enabled during add_host. How could it not be enabled in suspend? struct regulator *vmmc; /* Power regulator */ + bool restore_vmmc; /* Restore vmmc */ If you can explain the above, this should be turned into a SDHCI_-flag IMHO. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] mmc: sdhci-pci: Fix checkpatch.pl errors
Hi Ameya, On Tue, Apr 05, 2011 at 09:13:13PM +0300, Ameya Palande wrote: This patch fixes 21 errors and 6 warnings reported by checkpatch.pl Signed-off-by: Ameya Palande 2am...@gmail.com Looks okay to me. Have you checked that the binary does not differ? Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v2] mmc: sdhci: add error checking for mmc_add_host
On Mon, Apr 04, 2011 at 03:07:02PM +0900, Jaehoon Chung wrote: Sometimes we can't add the device,but we didn't check any error status. Need to check error status for mmc_add_host. Signed-off-by: Jaehoon Chung jh80.ch...@samsung.com Signed-off-by: kyungmin Park kyungmin.p...@samsung.com --- Please add here what changed since the last revision. You are still leaking the led_classdev. Why do you free the mmc_host when registering it failed? Looking at the remove function as comparison, there are still some buffers, timers, etc, forgotten :( Could you fix that, too, please? Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v2] mmc: sdhci: add error checking for mmc_add_host
registering it failed? Looking at the remove function as comparison, there are still some buffers, timers, etc, forgotten :( Could you fix that, too, please? To avoid misunderstandings, that sad smiley didn't mean you but the state of the current error-path in general. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] mmc: Add MPC512x SDHC driver.
Hi Vladimir, MPC512x have similar sdhc module as in MX2/MX3, but use different access to the registers. There are quite some drivers abstracting the register-access (sdhci-esdhc for example which is also used on ppc and arm). If the state-machine is the same, this really should be done. Otherwise maintenance will be harder. I have mx2/3-hardware here and am willing to test the changes. If the state-machine is significantly different, a seperate driver makes sense, of course. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] mmc: Add MPC512x SDHC driver.
I'll try to add abstraction. Great, thanks. But I can return to this only after our hardware will done (around one month). Okay, just set me to cc. Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH V8 3/4] mmc: sdhci-esdhc: make the writel/readl as the general APIs
On Fri, Mar 25, 2011 at 11:39:05AM +0800, Richard Zhu wrote: Add one flag to indicate the GPIO CD/WP is enabled or not on imx platforms, and reuse the writel/readl as the general APIs for imx SOCs. Signed-off-by: Richard Zhu hong-xing@freescale.com --- Please provide something like changes since last time _below_ this dashed line --- next time. Or use --cover-letter to introduce the changes. Other than that Reviewed-by: Wolfram Sang w.s...@pengutronix.de Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH V8 4/4] mmc: sdhci-esdhc: enable esdhc on imx53
On Fri, Mar 25, 2011 at 11:39:06AM +0800, Richard Zhu wrote: Fix the NO INT in the Multi-BLK IO in SD/MMC, and Multi-BLK read in SDIO on imx53. The CMDTYPE of the CMD register (offset 0xE) should be set to 11 when the STOP CMD12 is issued on imx53 to abort one open ended multi-blk IO. Otherwise one the TC INT wouldn't be generated. In exact block transfer, the controller doesn't complete the operations automatically as required at the end of the transfer and remains on hold if the abort command is not sent on imx53. As a result, the TC flag is not asserted and SW received timeout exeception. set bit1 of Vendor Spec registor to fix it. Signed-off-by: Richard Zhu hong-xing@freescale.com Signed-off-by: Richard Zhao richard.z...@freescale.com Reviewed-by: Wolfram Sang w.s...@pengutronix.de -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH V7 4/4] mmc: sdhci-esdhc: enable esdhc on imx53
Hi Richard, (please don't drop the list) I saw that some ones are discussing the implementation of the ACMD23 in the community. The new command introduced by SDHC spec3.0. This feature can solve the concern that mentioned by this patch. http://www.spinics.net/lists/linux-mmc/msg06428.html If the ACMD23 is implemented, the predefined-block number multi-block IO can be achieved without any confusion. It seems that the ACMD12 would be used in the another multi-block IO mode in those serial patches. So do you want to carry this patch until SDHC V3 is mainline and then change or do you prefer to skip this patch altogether? Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
