libertas_sdio and pxamci suspend issue
Hi, I'm currently updating the kernel of a PXA3xx platform that used to be bases on 3.0. On that, a libertas is connected to the PXA mci controller via SDIO. The card itself works fine on both the old version and 3.16, but on 3.16, I see the following error when trying to put the system to suspend: [ 13.459502] mmc0: new SDIO card at address 0001 [ 15.10] libertas_sdio: Libertas SDIO driver [ 15.107864] libertas_sdio: Copyright Pierre Ossman [ 15.660291] libertas_sdio mmc0:0001:1 (unregistered net_device): 00:19:88:21:f6:22, fw 9.70.20p0, cap 0x0303 [ 15.675960] libertas_sdio mmc0:0001:1 wlan0: Marvell WLAN 802.11 adapter [ 15.684075] cfg80211: Calling CRDA to update world regulatory domain # echo mem /sys/power/state [ 18.617456] PM: Syncing filesystems ... done. [ 18.634231] Freezing user space processes ... (elapsed 0.000 seconds) done. [ 18.641312] Freezing remaining freezable tasks ... (elapsed 0.007 seconds) done. [ 18.658322] dpm_run_callback(): pm_generic_suspend+0x0/0x38 returns -38 [ 18.665136] PM: Device mmc0:0001:1 failed to suspend: error -38 [ 18.671035] PM: Some devices failed to suspend, or early wake event detected The problem is caused by libertas' if_sdio_suspend() returning -ENOSYS here: /* If we're powered off anyway, just let the mmc layer remove the * card. */ if (!lbs_iface_active(card-priv)) return -ENOSYS; According to the comment, the code assumes the function is called from the mmc layer, which isn't the case - the stack trace leading to this function looks like this: [ 15.767603] [bf019048] (if_sdio_suspend [libertas_sdio]) from [c0285980] (pm_generic_suspend+0x2c/0x38) [ 15.779843] [c0285980] (pm_generic_suspend) from [c0286c1c] (dpm_run_callback.isra.21+0x18/0x44) [ 15.790863] [c0286c1c] (dpm_run_callback.isra.21) from [c0287cdc] (__device_suspend+0x148/0x220) [ 15.800039] [c0287cdc] (__device_suspend) from [c0288478] (dpm_suspend+0xa8/0x204) [ 15.808464] [c0288478] (dpm_suspend) from [c00410c8] (suspend_devices_and_enter+0x3c/0x300) [ 15.817858] [c00410c8] (suspend_devices_and_enter) from [c0041458] (pm_suspend+0xcc/0x1b0) [ 15.827037] [c0041458] (pm_suspend) from [c0040360] (state_store+0xb8/0xdc) [ 15.834678] [c0040360] (state_store) from [c022891c] (kobj_attr_store+0x14/0x20) [ 15.842664] [c022891c] (kobj_attr_store) from [c00ebcc0] (sysfs_kf_write+0x3c/0x48) [ 15.851209] [c00ebcc0] (sysfs_kf_write) from [c00eaed8] (kernfs_fop_write+0x100/0x158) [ 15.860039] [c00eaed8] (kernfs_fop_write) from [c009766c] (vfs_write+0xb4/0x188) [ 15.868335] [c009766c] (vfs_write) from [c0097bec] (SyS_write+0x3c/0x7c) [ 15.875949] [c0097bec] (SyS_write) from [c00092e0] (ret_fast_syscall+0x0/0x2c) Bisecting is tricky due to other details unfortunately. Any ideas? Thanks, Daniel -- 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: mwifiex card reset
Hi Bing, On 07/01/2014 08:44 AM, Bing Zhao wrote: Hence, we'll need some sort of internal API for this, and a phandle in dts. I wonder whether that glue logic might be better off living in the mmc core, as mwifiex might well be interfaced to other hosts? I may have missed something, but doesn't the MMC_POWER_OFF and MMC_POWER_ON|UP handling in controller driver help? Anyway the clocks/GPIOs/regulators are sort of platform dependent. Would it be better putting it in /arch/arm/mach-x/? Regulators are not platform specific, they never were. For GPIOs and clocks, we can simply depend on CONFIG_GPIO_GENERIC and CONFIG_COMMON_CLK. I wouldn't even bother to care for anything else. This way, we also get a way of referencing the resources in dts through phandles for free. What I was talking about above was that conducting the actual reset from the helper must be something that either the mmc core or individual host implementations can trigger. We need to repeat this action every time mwifiex decides to call its reset routine, which is currently implemented like this: mmc_remove_host(target); mdelay(20); mmc_add_host(target); But I haven't looked into possible ways of implementation yet. MMC_POWER_* might well be a suitable thing to hook into. Thanks, Daniel -- 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: mwifiex card reset
Hi Tony, everyone, Thanks Andreas for addressing this issue! So far, we've been using a terrible hack in the hsmmc in order to bring the card into a workable state, and we're looking for a nicer solution for awhile. On 06/30/2014 08:19 AM, Tony Lindgren wrote: * Andreas Fenkart afenk...@gmail.com [140629 12:43]: 2014-06-28 9:23 GMT+02:00 Tony Lindgren t...@atomide.com: * James Cameron qu...@laptop.org [140628 08:24]: Wouldn't it be best to have the mwifiex properly handle the reset GPIOs and idle status pins? doesn't work see ref 1) above OK so we can't do much anything at mwifiex probe time on SDIO bus because it won't probe unless the pins are configured. Maybe we should have a separate mwifiex-power helper module that just manages the reset/idle/regulator pins? Then mwifiex-reset can be always loaded and configure things so mwifiex-sdio can probe properly. And mwifiex-reset helper can also provide the user space interfaces for the reset/idle/regulator pins. Yes, a helper might be the best solution. It could even be a generic one, that just takes any number of clocks, reset GPIOs and regulators and takes care for sequencing them. However, we need to reference that helper from the mwifiex driver, for two reasons. a) we need to make sure the reset helper gets to do its job before the mwifiex driver scans the SDIO bus, and b) the reset helper needs to be called when the mmc host controller wants to do a card reset. Hence, we'll need some sort of internal API for this, and a phandle in dts. I wonder whether that glue logic might be better off living in the mmc core, as mwifiex might well be interfaced to other hosts? Thanks, Daniel -- 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 RESEND] mmc: omap_hsmmc: support more DT properties
This should probably be done implicitly through mmc_of_parse(), but that doesn't play well along with the multi-slot model the hsmmc driver features. Hence, for now, do it manually. The properties are already documented in Documentation/devicetree/bindings/mmc/mmc.txt. Signed-off-by: Daniel Mack zon...@gmail.com Acked-by: Balaji T K balaj...@ti.com --- This is a resend of a patch that was already acked by Balaji T K: http://www.spinics.net/lists/linux-mmc/msg25029.html drivers/mmc/host/omap_hsmmc.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 2815de6..a5a38cc 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1765,6 +1765,12 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev) if (of_find_property(np, ti,needs-special-hs-handling, NULL)) pdata-slots[0].features |= HSMMC_HAS_HSPE_SUPPORT; + if (of_find_property(np, keep-power-in-suspend, NULL)) + pdata-slots[0].pm_caps |= MMC_PM_KEEP_POWER; + + if (of_find_property(np, enable-sdio-wakeup, NULL)) + pdata-slots[0].pm_caps |= MMC_PM_WAKE_SDIO_IRQ; + return pdata; } #else -- 1.8.5.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] mmc: omap_hsmmc: support more DT properties
On 02/18/2014 03:36 PM, Balaji T K wrote: On Monday 17 February 2014 05:06 PM, Daniel Mack wrote: This should probably be done implicitly through mmc_of_parse(), but that doesn't play well along with the multi-slot model the hsmmc driver features. Hence, for now, do it manually. The properties are already documented in Documentation/devicetree/bindings/mmc/mmc.txt. Signed-off-by: Daniel Mack zon...@gmail.com looks good to me Acked-by: Balaji T K balaj...@ti.com Thanks for the review! Did it land in any tree yet? Best regards, Daniel --- drivers/mmc/host/omap_hsmmc.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 2815de6..a5a38cc 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1765,6 +1765,12 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev) if (of_find_property(np, ti,needs-special-hs-handling, NULL)) pdata-slots[0].features |= HSMMC_HAS_HSPE_SUPPORT; +if (of_find_property(np, keep-power-in-suspend, NULL)) +pdata-slots[0].pm_caps |= MMC_PM_KEEP_POWER; + +if (of_find_property(np, enable-sdio-wakeup, NULL)) +pdata-slots[0].pm_caps |= MMC_PM_WAKE_SDIO_IRQ; + return pdata; } #else -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mmc: omap_hsmmc: support more DT properties
This should probably be done implicitly through mmc_of_parse(), but that doesn't play well along with the multi-slot model the hsmmc driver features. Hence, for now, do it manually. The properties are already documented in Documentation/devicetree/bindings/mmc/mmc.txt. Signed-off-by: Daniel Mack zon...@gmail.com --- drivers/mmc/host/omap_hsmmc.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 2815de6..a5a38cc 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1765,6 +1765,12 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev) if (of_find_property(np, ti,needs-special-hs-handling, NULL)) pdata-slots[0].features |= HSMMC_HAS_HSPE_SUPPORT; + if (of_find_property(np, keep-power-in-suspend, NULL)) + pdata-slots[0].pm_caps |= MMC_PM_KEEP_POWER; + + if (of_find_property(np, enable-sdio-wakeup, NULL)) + pdata-slots[0].pm_caps |= MMC_PM_WAKE_SDIO_IRQ; + return pdata; } #else -- 1.8.5.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
[PATCH] mmc: omap_hsmmc: get capabilities from DT
When used in DT mode, the omap hsmmc driver has to take capabilites given in DT by calling mmc_of_parse(). Signed-off-by: Daniel Mack zon...@gmail.com --- drivers/mmc/host/omap_hsmmc.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 1865321..31337c1 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1907,7 +1907,15 @@ static int omap_hsmmc_probe(struct platform_device *pdev) if (mmc_slot(host).nonremovable) mmc-caps |= MMC_CAP_NONREMOVABLE; - mmc-pm_caps = mmc_slot(host).pm_caps; + if (pdev-dev.of_node) { + ret = mmc_of_parse(mmc); + if (ret 0) { + dev_err(pdev-dev, mmc_of_parse() failed: %d\n, ret); + goto err_irq; + } + } else { + mmc-pm_caps = mmc_slot(host).pm_caps; + } omap_hsmmc_conf_bus_power(host); -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ARM: AM335x: Kernel oops when using EDMA and MMC
Hi Balaji, On 18.07.2013 18:40, Balaji T K wrote: With DMA channel info retrieved from dt binding on 3.11rc1, unused_chan_list is broken after hwmod cleanup removing mmc sdma resource info, hence pdev resource wont have DMA resource populated. arch/arm/common/edma.c static int prepare_unused_channel_list(struct device *dev, void *data) { struct platform_device *pdev = to_platform_device(dev); int i, ctlr; for (i = 0; i pdev-num_resources; i++) { if ((pdev-resource[i].flags IORESOURCE_DMA) (int)pdev-resource[i].start = 0) { ctlr = EDMA_CTLR(pdev-resource[i].start); clear_bit(EDMA_CHAN_SLOT(pdev-resource[i].start), edma_cc[ctlr]-edma_unused); } } return 0; } int edma_alloc_channel(int channel, if (!unused_chan_list_done) { /* * Scan all the platform devices to find out the EDMA channels * used and clear them in the unused list, making the rest * available for ARM usage. */ ret = bus_for_each_dev(platform_bus_type, NULL, NULL, prepare_unused_channel_list); if (ret 0) return ret; unused_chan_list_done = true; } === with the below hack patch, edma is working fine with mmc on your 3.11rc1+ branch diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c index a432e6c..5a19164 100644 --- a/arch/arm/common/edma.c +++ b/arch/arm/common/edma.c @@ -1262,8 +1262,8 @@ int edma_start(unsigned channel) if (test_bit(channel, edma_cc[ctlr]-edma_unused)) { pr_debug(EDMA: ESR%d %08x\n, j, edma_shadow0_read_array(ctlr, SH_ESR, j)); - edma_shadow0_write_array(ctlr, SH_ESR, j, mask); - return 0; +// edma_shadow0_write_array(ctlr, SH_ESR, j, mask); +// return 0; } /* EDMA channel with event association */ Yes, this in fact works for me as well. Thanks for the quick reply! What would be the proper fix for this? Best, Daniel -- 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] ARM: mach-moxart: platform port for MOXA ART SoC
Hi Jonas, On 13.03.2013 16:37, Jonas Jensen wrote: I ask for feedback and to submit (if possible) a new ARM SoC platform port. This is now near complete (I think) (tested on UC-7112-LX Plus) and applies to 2.6.34.14. First of all - thanks for submitting to the upstream kernel! However, your patch has many severe problems which you need to address. * please rebase your work. 2.6.34 is almost three years old now. 3.9 is in it's stabilisation phase, and all new support has to be done for 3.10. * all new platforms must be written with device-tree support * all drivers must have device-tree support as well The patch contains the following drivers and platform specific implementations: * ARCH_MOXART (FA526 processor) * 100Hz interrupt timer * UART * MTD map driver * Ethernet driver (RTL8201CP) * MMC driver * MOXA Smartio/Industio family multiport serial driver * RTC driver * Watchdog driver * GPIO driver Never send one big patch but series of smaller ones, so the individual subsystem maintainers can review and approve their bits. Please also read Documentation/SubmittingPatches for a lot more information about this subject. Best regards, Daniel Predicted patch rejects below (in need of a solution, feedback is much appreciated) because they are critical in areas of boot, MMC and TTY. arch/arm/boot/compressed/head.S: A valid (and unique) architecture ID is not loaded to r1. Looks like the bootloader is broken, it should be doing this! http://gicl.cs.drexel.edu/people/sevy/linux/ARM_Linux_boot_sequence.html arch/arm/tools/mach-types: Omitted (do not edit manually / add a new machine using http://www.arm.linux.org.uk/developer/machines/?action=new). A fix to this and above is not feasible as long as MOXA withholds bootloader sources (requested without success). drivers/char/mxser.c drivers/char/mxser.h: MOXA SMARTIO/INDUSTIO/INTELLIO SERIAL CARD (Jiří Slabý): Force board setup for CONFIG_ARCH_MOXART. ASYNCB_CLOSING is avoided because of a lockup (infinite wait after tty_wait_until_sent). Why this happens is unknown (to me) I'm hoping someone (Jiří?) can shed light. SysRq trace @ http://ideone.com/e845mr What significance does ASYNCB_CLOSING have? Obviously, automatic detection is better but mxser_read_register is pointless on this hardware. What to do instead? Is it better to make a copy and submit a new driver? drivers/mmc/core/sd.c: The MMC controller is special? UNSTUFF_BITS is redefined here http://repo.or.cz/w/linux-2.6.19-moxart.git/blob/50cdf2c57662f9f69c5615976412f76bfd73311a:/drivers/mmc/mmc.c . Without the new macro it'll report the wrong geometry and prod_name. I'm thinking a driver should never have to redefine UNSTUFF_BITS. Possible workaround: modify bits (in driver) to line up as expected before returning the response (mmc_request_done). For reference, this is my previous post from a few months back: http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/137130.html Gitweb: http://repo.or.cz/w/linux-2.6.34.14-moxart.git/commitdiff/?h=3bc2e98ebb92961e1c5992736186920cd070f4eehp=b7f1d43323eceb02fd663a71eb2f8be9c17e6740 Download link (size: 193K): https://linux-2-6-34-14-moxart.googlecode.com/files/linux-2.6.34.14-moxart.patch -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: omap_hsmmc: Enable SDIO IRQ using a GPIO in idle mode.
On 10.01.2013 21:22, Tony Lindgren wrote: * Andreas Fenkart andreas.fenk...@streamunlimited.com [121220 14:15]: Without functional clock the omap_hsmmc module can't forward SDIO IRQs to the system. This patch reconfigures dat1 line as a gpio while the fclk is off. And uses SDIO IRQ detection of the module, while fclk is present. Looks pretty good to me, however I could not figure out what to apply this on for testing. It fails to apply at least against current linux next, can you please update against that? +static void omap_hsmmc_enable_sdio_irq(struct mmc_host *mmc, int enable) +{ +struct omap_hsmmc_host *host = mmc_priv(mmc); +u32 irq_mask; +unsigned long flags; + +spin_lock_irqsave(host-irq_lock, flags); + +host-sdio_irq_en = (enable != 0) ? true : false; + +if (host-active_pinmux) { +irq_mask = OMAP_HSMMC_READ(host-base, ISE); +if (enable) +irq_mask |= CIRQ_ENABLE; +else +irq_mask = ~CIRQ_ENABLE; +OMAP_HSMMC_WRITE(host-base, IE, irq_mask); + +if (!host-req_in_progress) +OMAP_HSMMC_WRITE(host-base, ISE, irq_mask); + +#if 0 +OMAP_HSMMC_READ(host-base, IE); /* flush posted write */ +#endif Maybe just replace #if 0 with just a comment in case it turns out to be needed for some cases? Is there any update on this series? Andreas, did you do more tests? Thanks and best regards, Daniel -- 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: omap_hsmmc: SDIO IRQ on AM335x family
Hi Tony, Hi Andreas, On 30.11.2012 18:40, Tony Lindgren wrote: * Andreas Fenkart andreas.fenk...@streamunlimited.com [121130 03:21]: The alternative was to configure dat1 line as a GPIO, while waiting for an IRQ. Then configuring it back as dat1 when the SDIO card is signalling an IRQ. Or the host starts a transfer. I guess this will perform poorly, hence not considering it really. This might work for SDIO cards. It should be disabled for data cards naturally to avoid potential data corruption. The way to implement this is set named states in the .dts file for the pins using pinctrl-single.c, then have the MMC driver request states default active and idle during the probe, then toggle between active and idle during the runtime. As far as I remember the GPIO functionality does not need to be enabled, just muxing the pin to GPIO mode for the wake-up is enough. Wouldn't that be racy, given that an interrupt which occurs at beween the point in time when the driver decides to wait for IRQs again until the mux has finished switching over, could potentially be lost? If there is another way to solve that cleanly on at least some hardware derivats, I'd say that should be preferred and be supported by the driver. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] MMC: omap_hsmmc: set platform data after probe from DT node
Hi Grant, On 13.10.2012 02:05, Grant Likely wrote: Balaji T K balaj...@ti.com wrote: On Friday 12 October 2012 08:44 PM, Daniel Mack wrote: On 12.10.2012 16:56, Balaji T K wrote: On Friday 12 October 2012 07:59 PM, Daniel Mack wrote: On 12.10.2012 12:58, Daniel Mack wrote: diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 19ccb59..4b70823 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1728,6 +1728,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev) const u16 *offsetp = match-data; pdata-reg_offset = *offsetp; } +pdev-dev.platform_data = pdata; } if (pdata == NULL) { FWIW, this is the Oops I see without this patch: Hi, Shouldn't pdev-dev.platform_data be set to NULL on _remove ? Why? To make sure on second insmod it is NULL, When built as module, So that of_get_hsmmc_pdata is called to create pdata. Actually the driver should *never* modify the value of dev-platform_data. Ever. That's interesting, because many drivers do this, especially since they were converted to DT probing. Mostly because that way, the platform data logic in callback functions can be reused, and often the platform specific data is only stored in pdata and taken from there during the lifetime of a device. Is there any particular reason why this approach is frowned upon? Make a copy instead. A copy of what exactly? Of all members of the legacy pdata you mean? Thanks, Daniel -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] MMC: omap_hsmmc: set platform data after probe from DT node
On 13.10.2012 10:48, Grant Likely wrote: On Sat, Oct 13, 2012 at 9:05 AM, Daniel Mack zon...@gmail.com wrote: Hi Grant, On 13.10.2012 02:05, Grant Likely wrote: Balaji T K balaj...@ti.com wrote: On Friday 12 October 2012 08:44 PM, Daniel Mack wrote: On 12.10.2012 16:56, Balaji T K wrote: On Friday 12 October 2012 07:59 PM, Daniel Mack wrote: On 12.10.2012 12:58, Daniel Mack wrote: diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 19ccb59..4b70823 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1728,6 +1728,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev) const u16 *offsetp = match-data; pdata-reg_offset = *offsetp; } +pdev-dev.platform_data = pdata; } if (pdata == NULL) { FWIW, this is the Oops I see without this patch: Hi, Shouldn't pdev-dev.platform_data be set to NULL on _remove ? Why? To make sure on second insmod it is NULL, When built as module, So that of_get_hsmmc_pdata is called to create pdata. Actually the driver should *never* modify the value of dev-platform_data. Ever. That's interesting, because many drivers do this, especially since they were converted to DT probing. Mostly because that way, the platform data logic in callback functions can be reused, and often the platform specific data is only stored in pdata and taken from there during the lifetime of a device. Is there any particular reason why this approach is frowned upon? Yes. The platform data pointer is owned by the code that registered the platform device, not by the device driver. Some drivers do it, but it is definitely illegal. I should add code to the platform bus core code to throw a warning to any drivers that do that. It is a problem because it becomes easy to mess up the lifetime model of device data, particularly when it comes to unbinding/rebinding devices. For example, if a driver changes the pdata pointer and then gets unbound, then there will be a stale pdata pointer that may point to either incorrect data or a data structure that is no longer allocated. You could argue that it is fine if the driver is smart about how it cleans up after itself, but in my experience driver authors rarely get it correct and it results in a lot more code than is necessary. It is far better for the driver to either grab all the data it needs out of pdata at .probe() time, or to keep a copy of the 'correct' pdata (correct depending on where the device retrieved it's data) in it's private data structure instead of modifying the device. Ok, interesting. Thanks for writing this up. Make a copy instead. A copy of what exactly? Of all members of the legacy pdata you mean? Yes. If the driver directly accesses the pdata structure outside of the .probe() hook, then it should be modified to either copy the pdata into the driver's private data structure, or it should copy the pointer to the pdata so that OF usage can allocate one itself. For example: somedriver_probe(struct platform_device *pdev) { struct somedriver_private *somedriver; somedriver = devm_kzalloc(sizeof (*somedriver), GFP_KERNEL); somedriver-pdata = pdev-platform_data; if (OF) somedriver-pdata = devm_kzalloc(sizeof (*somedriver-pdata), GFP_KERNEL); } The bonus with using devm_kzalloc is the driver doesn't even need to do anything special to undo these allocations on failure or release. :-) Ok, understood. Will keep an eye on this in the future. Thanks again for the explanation. For this particular driver, this means that both my and Balaji's ways of fixing this are wrong? Daniel -- 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] MMC: some omap_hsmmc fixes
Here are some assorted patches for the omap_hsmmc driver that I need on top Linus' current development branch to make it work on a AM33xx board. 1/4 and 2/4 qualify as bug fixes and I'm puzzled that these didn't hit anyone else yet. Daniel Mack (4): MMC: omap_hsmmc: set platform data after probe from DT node MMC: omap_hsmmc: fix DMA config block MMC: omap_hsmmc: claim pinctrl at probe time MMC: omap_hsmmc: add DT property for max bus frequency drivers/mmc/host/omap_hsmmc.c | 19 --- 1 file changed, 16 insertions(+), 3 deletions(-) -- 1.7.11.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 1/4] MMC: omap_hsmmc: set platform data after probe from DT node
When probed from DT, the self-allocated platform data has to be attached to the actual device. Otherwise a NULL pointer will be dereferenced from omap_hsmmc_card_detect if a gpio handle for card-detect has been passed. Signed-off-by: Daniel Mack zon...@gmail.com Cc: Venkatraman S svenk...@ti.com Cc: Chris Ball c...@laptop.org Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: linux-o...@vger.kernel.org --- drivers/mmc/host/omap_hsmmc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 19ccb59..4b70823 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1728,6 +1728,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev) const u16 *offsetp = match-data; pdata-reg_offset = *offsetp; } + pdev-dev.platform_data = pdata; } if (pdata == NULL) { -- 1.7.11.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/4] MMC: omap_hsmmc: fix DMA config block
The .direction field of the config passed to dmaengine_slave_config() must be set in order to make the generic code store the configured bus width. Without this patch, I keep getting the following error on a AM33xx board: [1.963144] edma-dma-engine edma-dma-engine.0: Undefined slave buswidth [1.970158] omap_hsmmc mmc.4: prep_slave_sg() failed [1.975454] mmc1: error -1 whilst initialising SD card Signed-off-by: Daniel Mack zon...@gmail.com Cc: Venkatraman S svenk...@ti.com Cc: Chris Ball c...@laptop.org Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: linux-o...@vger.kernel.org Cc: Matt Porter mpor...@ti.com Cc: Russell King rmk+ker...@arm.linux.org.uk --- drivers/mmc/host/omap_hsmmc.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 4b70823..2c57b0d 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1260,6 +1260,8 @@ static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host, cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; cfg.src_maxburst = data-blksz / 4; cfg.dst_maxburst = data-blksz / 4; + cfg.direction = data-flags MMC_DATA_WRITE ? + DMA_MEM_TO_DEV : DMA_DEV_TO_MEM; ret = dmaengine_slave_config(chan, cfg); if (ret) @@ -1270,8 +1272,8 @@ static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host, return ret; tx = dmaengine_prep_slave_sg(chan, data-sg, data-sg_len, - data-flags MMC_DATA_WRITE ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM, - DMA_PREP_INTERRUPT | DMA_CTRL_ACK); +cfg.direction, +DMA_PREP_INTERRUPT | DMA_CTRL_ACK); if (!tx) { dev_err(mmc_dev(host-mmc), prep_slave_sg() failed\n); /* FIXME: cleanup */ -- 1.7.11.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 4/4] MMC: omap_hsmmc: add DT property for max bus frequency
Maximum bus frequency can be limited by external circuitry like level shifters etc. Allow passing this value from DT. Signed-off-by: Daniel Mack zon...@gmail.com Cc: Venkatraman S svenk...@ti.com Cc: Chris Ball c...@laptop.org Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: linux-o...@vger.kernel.org --- drivers/mmc/host/omap_hsmmc.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 86f0759..4650ef7 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1676,7 +1676,7 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev) { struct omap_mmc_platform_data *pdata; struct device_node *np = dev-of_node; - u32 bus_width; + u32 bus_width, max_freq; pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); if (!pdata) @@ -1703,6 +1703,9 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev) if (of_find_property(np, ti,needs-special-reset, NULL)) pdata-slots[0].features |= HSMMC_HAS_UPDATED_RESET; + if (!of_property_read_u32(np, max-frequency, max_freq)) + pdata-max_freq = max_freq; + return pdata; } #else -- 1.7.11.4 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] MMC: omap_hsmmc: set platform data after probe from DT node
On 12.10.2012 12:58, Daniel Mack wrote: When probed from DT, the self-allocated platform data has to be attached to the actual device. Otherwise a NULL pointer will be dereferenced from omap_hsmmc_card_detect if a gpio handle for card-detect has been passed. Signed-off-by: Daniel Mack zon...@gmail.com Cc: Venkatraman S svenk...@ti.com Cc: Chris Ball c...@laptop.org Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: linux-o...@vger.kernel.org --- drivers/mmc/host/omap_hsmmc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 19ccb59..4b70823 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1728,6 +1728,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev) const u16 *offsetp = match-data; pdata-reg_offset = *offsetp; } + pdev-dev.platform_data = pdata; } if (pdata == NULL) { FWIW, this is the Oops I see without this patch: [1.492727] Unable to handle kernel NULL pointer dereference at virtual address 0040 [1.501327] pgd = c0004000 [1.504166] [0040] *pgd= [1.507993] Internal error: Oops: 5 [#1] SMP THUMB2 [1.513100] Modules linked in: [1.516315] CPU: 0Not tainted (3.6.0-10656-g4bc7e4d-dirty #75) [1.522890] PC is at omap_hsmmc_card_detect+0x6/0x18 [1.528090] LR is at omap_hsmmc_get_cd+0x1d/0x24 [1.532929] pc : [c025a3ee]lr : [c0259fed]psr: 4133 [1.532929] sp : cf1b5e78 ip : 0c288145 fp : 0002 [1.544939] r10: cf1b4000 r9 : cf1b5eb8 r8 : [1.550408] r7 : cf1a4600 r6 : 6113 r5 : cf33ba98 r4 : cf33b800 [1.557240] r3 : r2 : r1 : r0 : cf0d1c10 [1.564075] Flags: nZcv IRQs on FIQs on Mode SVC_32 ISA Thumb Segment kernel [1.571906] Control: 50c5387d Table: 80004019 DAC: 0015 [1.577921] Process kworker/u:1 (pid: 19, stack limit = 0xcf1b4240) [1.584480] Stack: (0xcf1b5e78 to 0xcf1b6000) [1.589045] 5e60: c025a3e9 c0259fed [1.597611] 5e80: c0259fd1 c024f959 cf33bb0c cf1a5dc0 c05c87c0 c003a42f 0002 [1.606176] 5ea0: c003a3d8 c005e181 cf1b35c0 c024f829 c0adfe68 c0687048 [1.614741] 5ec0: c0494f78 c05c8780 cf1a5dc0 c05c88e8 cf1b4000 c05c88f0 cf1a5dd0 [1.623306] 5ee0: c0531f40 c053d678 c05c8780 c003c2ab cf1b5f18 c005e935 c05c87c0 c0531f40 [1.631872] 5f00: c0531f40 c7e3f1f9 cf1b5f34 cf069e20 cf1b5f34 cf1a5dc0 c003c191 [1.640436] 5f20: c003f809 cf1a5dc0 [1.649001] 5f40: dead4ead c05c8ef8 [1.657565] 5f60: c0455c08 cf1b5f64 cf1b5f64 dead4ead [1.666130] 5f80: c05c8ef8 c0455c08 cf1b5f90 cf1b5f90 cf069e20 c003f791 [1.674694] 5fa0: c000d1e1 aaa3aaab b1ba aae3ea32 aba2a3aa [1.683260] 5fc0: baaababe bbbaaaba aa2b bb2e2aba aabaa2bb aaabb2bb aaabbaf3 f3bb2aaa [1.691824] 5fe0: aabaabba 23ae2beb a2afab2a b2a2ab2a 0013 aaabaaab bfaf a0a2 [1.700399] [c025a3ee] (omap_hsmmc_card_detect+0x6/0x18) from [c0259fed] (omap_hsmmc_get_cd+0x1d/0x24) [1.710528] [c0259fed] (omap_hsmmc_get_cd+0x1d/0x24) from [c024f959] (mmc_rescan+0x131/0x1e4) [1.719842] [c024f959] (mmc_rescan+0x131/0x1e4) from [c003a42f] (process_one_work+0x137/0x37c) [1.729234] [c003a42f] (process_one_work+0x137/0x37c) from [c003c2ab] (worker_thread+0x11b/0x364) [1.738899] [c003c2ab] (worker_thread+0x11b/0x364) from [c003f809] (kthread+0x79/0x84) [1.747572] [c003f809] (kthread+0x79/0x84) from [c000d1e1] (ret_from_kernel_thread+0xd/0x14) [1.756773] Code: bf00 b508 f8d0 3084 (6c18) f75f [1.761943] ---[ end trace b94c447c4835422d ]--- -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] MMC: omap_hsmmc: set platform data after probe from DT node
On 12.10.2012 16:56, Balaji T K wrote: On Friday 12 October 2012 07:59 PM, Daniel Mack wrote: On 12.10.2012 12:58, Daniel Mack wrote: When probed from DT, the self-allocated platform data has to be attached to the actual device. Otherwise a NULL pointer will be dereferenced from omap_hsmmc_card_detect if a gpio handle for card-detect has been passed. Signed-off-by: Daniel Mack zon...@gmail.com Cc: Venkatraman S svenk...@ti.com Cc: Chris Ball c...@laptop.org Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: linux-o...@vger.kernel.org --- drivers/mmc/host/omap_hsmmc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 19ccb59..4b70823 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1728,6 +1728,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev) const u16 *offsetp = match-data; pdata-reg_offset = *offsetp; } + pdev-dev.platform_data = pdata; } if (pdata == NULL) { FWIW, this is the Oops I see without this patch: Hi, Shouldn't pdev-dev.platform_data be set to NULL on _remove ? Why? BTW, I posted a patch for the same by accessing saved version from host-pdata http://permalink.gmane.org/gmane.linux.kernel.mmc/16996 Ok, that's another solution. I thought about this too, but then chose the easier way :) I don't care which patch is taken, as long as we have a fix in mainline. Thanks, Daniel -- 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 4/4] MMC: omap_hsmmc: add DT property for max bus frequency
On 12.10.2012 17:25, Rob Herring wrote: On 10/12/2012 05:58 AM, Daniel Mack wrote: Maximum bus frequency can be limited by external circuitry like level shifters etc. Allow passing this value from DT. Signed-off-by: Daniel Mack zon...@gmail.com Cc: Venkatraman S svenk...@ti.com Cc: Chris Ball c...@laptop.org Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: linux-o...@vger.kernel.org --- drivers/mmc/host/omap_hsmmc.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 86f0759..4650ef7 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1676,7 +1676,7 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev) { struct omap_mmc_platform_data *pdata; struct device_node *np = dev-of_node; -u32 bus_width; +u32 bus_width, max_freq; pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); if (!pdata) @@ -1703,6 +1703,9 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev) if (of_find_property(np, ti,needs-special-reset, NULL)) pdata-slots[0].features |= HSMMC_HAS_UPDATED_RESET; +if (!of_property_read_u32(np, max-frequency, max_freq)) +pdata-max_freq = max_freq; + Is this property documented? Yes, in the generic part. Forgot to mention that. Documentation/devicetree/bindings/mmc/mmc.txt -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/3] DaVinci DMA engine conversion
Hi Matt, On 27.08.2012 17:33, Daniel Mack wrote: On 23.08.2012 03:09, Matt Porter wrote: This series begins the conversion of the DaVinci private EDMA API implementation to a DMA engine driver and converts two of the three in-kernel users of the private EDMA API to DMA engine. The approach taken is similar to the recent OMAP DMA Engine conversion. The EDMA DMA Engine driver is a wrapper around the existing private EDMA implementation and registers the platform device within the driver. This allows the conversion series to stand alone with just the drivers and no changes to platform code. It also allows peripheral drivers to continue to use the private EDMA implementation until they are converted. The EDMA DMA Engine driver supports slave transfers only at this time. It is planned to add cyclic transfers in support of audio peripherals. There are three users of the private EDMA API in the kernel now: davinci_mmc, spi-davinci, and davinci-mcasp. This series provides DMA Engine conversions for the davinci_mmc and spi-davinci drivers which use the supported slave transfers. This series has been tested on an AM18x EVM and Hawkboard with driver performance comparable to that of the private EDMA API implementations. Both MMC0 and MMC1 are tested which handles the DA850/OMAP-L138/AM18x specific case where MMC1 uses DMA channels on a second EDMA channel controller. All other platforms have a simpler design with just a single EDMA channel controller. For those wanting to easily test this series, I've pushed a branch for each version to my github tree at https://github.com/ohporter/linux. The current branch is edma-dmaengine-v3. After this series, the current plan is to complete the mcasp driver conversion which includes adding cyclic dma support. This will then enable the removal and refactoring of the private EDMA API functionality into the EDMA DMA Engine driver. Since EDMA is also used on the AM33xx family of parts in mach-omap2/, the plan is to enable this driver on that platform as well. Once you have a patch for the McASP driver conversion, I can happily test this on a AM33xx board, together with Gururaja's latest McASP refactoring series. Let me know how I can help you here. Did you find some time yet to continue on this side? I don't want to appear pushy, but as I need to finish some DT transition on a AM33xx-based board, I would much like to help out here, in case I can do anything to help speed things along. Many thanks for your work! Daniel -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/3] DaVinci DMA engine conversion
Hi Matt, On 23.08.2012 03:09, Matt Porter wrote: This series begins the conversion of the DaVinci private EDMA API implementation to a DMA engine driver and converts two of the three in-kernel users of the private EDMA API to DMA engine. The approach taken is similar to the recent OMAP DMA Engine conversion. The EDMA DMA Engine driver is a wrapper around the existing private EDMA implementation and registers the platform device within the driver. This allows the conversion series to stand alone with just the drivers and no changes to platform code. It also allows peripheral drivers to continue to use the private EDMA implementation until they are converted. The EDMA DMA Engine driver supports slave transfers only at this time. It is planned to add cyclic transfers in support of audio peripherals. There are three users of the private EDMA API in the kernel now: davinci_mmc, spi-davinci, and davinci-mcasp. This series provides DMA Engine conversions for the davinci_mmc and spi-davinci drivers which use the supported slave transfers. This series has been tested on an AM18x EVM and Hawkboard with driver performance comparable to that of the private EDMA API implementations. Both MMC0 and MMC1 are tested which handles the DA850/OMAP-L138/AM18x specific case where MMC1 uses DMA channels on a second EDMA channel controller. All other platforms have a simpler design with just a single EDMA channel controller. For those wanting to easily test this series, I've pushed a branch for each version to my github tree at https://github.com/ohporter/linux. The current branch is edma-dmaengine-v3. After this series, the current plan is to complete the mcasp driver conversion which includes adding cyclic dma support. This will then enable the removal and refactoring of the private EDMA API functionality into the EDMA DMA Engine driver. Since EDMA is also used on the AM33xx family of parts in mach-omap2/, the plan is to enable this driver on that platform as well. Once you have a patch for the McASP driver conversion, I can happily test this on a AM33xx board, together with Gururaja's latest McASP refactoring series. Let me know how I can help you here. Thanks, Daniel -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ARM: OMAP: hsmmc: add max_freq field
External circuitry like level shifters may limit the maximum operation speed of the hsmmc controller. Add a field to struct omap2_hsmmc_info so boards can adjust the setting on demand. Signed-off-by: Daniel Mack zon...@gmail.com Acked-by: Tony Lindgren t...@atomide.com Cc: Chris Ball c...@laptop.org Cc: Cousson, Benoit b-cous...@ti.com --- This trivial patch is in my queue for some time already, and Tony acked on it awhile back. I was told to take Chris Ball in the loop as there is currently no OMAP MMC maintainer. This still applies cleanly to 3.3-rc4. Chris, can you queue this? arch/arm/mach-omap2/hsmmc.c |1 + arch/arm/mach-omap2/hsmmc.h |2 ++ drivers/mmc/host/omap_hsmmc.c |8 ++-- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-omap2/hsmmc.c b/arch/arm/mach-omap2/hsmmc.c index f4a1020..4c7bc36 100644 --- a/arch/arm/mach-omap2/hsmmc.c +++ b/arch/arm/mach-omap2/hsmmc.c @@ -298,6 +298,7 @@ static int __init omap_hsmmc_pdata_init(struct omap2_hsmmc_info *c, mmc-slots[0].caps = c-caps; mmc-slots[0].internal_clock = !c-ext_clock; mmc-dma_mask = 0x; + mmc-max_freq = c-max_freq; if (cpu_is_omap44xx()) mmc-reg_offset = OMAP4_MMC_REG_OFFSET; else diff --git a/arch/arm/mach-omap2/hsmmc.h b/arch/arm/mach-omap2/hsmmc.h index f757e78..65a8c12 100644 --- a/arch/arm/mach-omap2/hsmmc.h +++ b/arch/arm/mach-omap2/hsmmc.h @@ -25,6 +25,8 @@ struct omap2_hsmmc_info { char*name; /* or NULL for default */ struct device *dev; /* returned: pointer to mmc adapter */ int ocr_mask; /* temporary HACK */ + int max_freq; /* maximum clock, if constrained by external +* circuitry, or 0 for default */ /* Remux (pad configuration) when powering on/off */ void (*remux)(struct device *dev, int slot, int power_on); /* init some special card */ diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 101cd31..8215ef9 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1927,8 +1927,12 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev) if (mmc_slot(host).vcc_aux_disable_is_sleep) mmc_slot(host).no_off = 1; - mmc-f_min = OMAP_MMC_MIN_CLOCK; - mmc-f_max = OMAP_MMC_MAX_CLOCK; + mmc-f_min = OMAP_MMC_MIN_CLOCK; + + if (pdata-max_freq 0) + mmc-f_max = pdata-max_freq; + else + mmc-f_max = OMAP_MMC_MAX_CLOCK; spin_lock_init(host-irq_lock); -- 1.7.7.4 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP: hsmmc: add max_freq field
ping? Could anyone care for queueing this please? On Thu, Dec 29, 2011 at 2:22 PM, Daniel Mack zon...@gmail.com wrote: On 12/23/2011 04:40 PM, T Krishnamoorthy, Balaji wrote: On Wed, Dec 14, 2011 at 6:52 PM, Daniel Mack zon...@gmail.com wrote: diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 101cd31..8215ef9 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1927,8 +1927,12 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev) if (mmc_slot(host).vcc_aux_disable_is_sleep) mmc_slot(host).no_off = 1; - mmc-f_min = OMAP_MMC_MIN_CLOCK; - mmc-f_max = OMAP_MMC_MAX_CLOCK; + mmc-f_min = OMAP_MMC_MIN_CLOCK; Stray change for f_min ? No, this was intended. The indentation doesn't make sense anymore when not grouped with the f_max assignment. Other than that, what is necessary to get this picked? Tony? :) Thanks, Daniel -- 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] ARM: OMAP: hsmmc: add max_freq field
On 12/23/2011 04:40 PM, T Krishnamoorthy, Balaji wrote: On Wed, Dec 14, 2011 at 6:52 PM, Daniel Mack zon...@gmail.com wrote: diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 101cd31..8215ef9 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1927,8 +1927,12 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev) if (mmc_slot(host).vcc_aux_disable_is_sleep) mmc_slot(host).no_off = 1; - mmc-f_min = OMAP_MMC_MIN_CLOCK; - mmc-f_max = OMAP_MMC_MAX_CLOCK; + mmc-f_min = OMAP_MMC_MIN_CLOCK; Stray change for f_min ? No, this was intended. The indentation doesn't make sense anymore when not grouped with the f_max assignment. Other than that, what is necessary to get this picked? Tony? :) Thanks, Daniel -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ARM: OMAP: hsmmc: add max_freq field
External circuitry like level shifters may limit the maximum operation speed of the hsmmc controller. Add a field to struct omap2_hsmmc_info so boards can adjust the setting on demand. Signed-off-by: Daniel Mack zon...@gmail.com Cc: Tony Lindgren t...@atomide.com --- arch/arm/mach-omap2/hsmmc.c |1 + arch/arm/mach-omap2/hsmmc.h |2 ++ drivers/mmc/host/omap_hsmmc.c |8 ++-- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-omap2/hsmmc.c b/arch/arm/mach-omap2/hsmmc.c index f4a1020..4c7bc36 100644 --- a/arch/arm/mach-omap2/hsmmc.c +++ b/arch/arm/mach-omap2/hsmmc.c @@ -298,6 +298,7 @@ static int __init omap_hsmmc_pdata_init(struct omap2_hsmmc_info *c, mmc-slots[0].caps = c-caps; mmc-slots[0].internal_clock = !c-ext_clock; mmc-dma_mask = 0x; + mmc-max_freq = c-max_freq; if (cpu_is_omap44xx()) mmc-reg_offset = OMAP4_MMC_REG_OFFSET; else diff --git a/arch/arm/mach-omap2/hsmmc.h b/arch/arm/mach-omap2/hsmmc.h index f757e78..65a8c12 100644 --- a/arch/arm/mach-omap2/hsmmc.h +++ b/arch/arm/mach-omap2/hsmmc.h @@ -25,6 +25,8 @@ struct omap2_hsmmc_info { char*name; /* or NULL for default */ struct device *dev; /* returned: pointer to mmc adapter */ int ocr_mask; /* temporary HACK */ + int max_freq; /* maximum clock, if constrained by external +* circuitry, or 0 for default */ /* Remux (pad configuration) when powering on/off */ void (*remux)(struct device *dev, int slot, int power_on); /* init some special card */ diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 101cd31..8215ef9 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1927,8 +1927,12 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev) if (mmc_slot(host).vcc_aux_disable_is_sleep) mmc_slot(host).no_off = 1; - mmc-f_min = OMAP_MMC_MIN_CLOCK; - mmc-f_max = OMAP_MMC_MAX_CLOCK; + mmc-f_min = OMAP_MMC_MIN_CLOCK; + + if (pdata-max_freq 0) + mmc-f_max = pdata-max_freq; + else + mmc-f_max = OMAP_MMC_MAX_CLOCK; spin_lock_init(host-irq_lock); -- 1.7.7.4 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Oops on suspend with libertas SDIO (Linux 3.2-rc2)
On Thu, Nov 17, 2011 at 4:49 PM, Chris Ball c...@laptop.org wrote: Hi, On Thu, Nov 17 2011, Sven Neumann wrote: I've given 3.2-rc2 a try today and the kernel oopsed when going into suspend: [ 20.387912] Freezing user space processes ... (elapsed 0.01 seconds) done. [ 20.408759] Freezing remaining freezable tasks ... (elapsed 0.01 seconds) done. [ 20.432966] libertas_sdio mmc0:0001:1: mmc0:0001:1: suspend: PM flags = 0x0 [ 20.439978] libertas_sdio mmc0:0001:1: Suspend without wake params -- powering down card [ 23.456061] libertas_sdio mmc0:0001:1: wlan0: command 0x0010 timed out [ 23.462626] libertas_sdio mmc0:0001:1: wlan0: Timeout submitting command 0x0010 [ 23.470026] libertas_sdio: Resetting card... [ 23.474140] [ cut here ] [ 23.479002] WARNING: at fs/sysfs/group.c:138 sysfs_remove_group+0x50/0x11c() [ 23.486045] sysfs group c05cb5c8 not found for kobject 'mmc0:0001:1' [ 23.492358] Modules linked in: libertas_sdio libertas pxamci [ 23.498091] [c000dbb4] (unwind_backtrace+0x0/0xec) from [c001ca10] (warn_slowpath_common+0x4c/0x64) [ 23.507467] [c001ca10] (warn_slowpath_common+0x4c/0x64) from [c001cabc] (warn_slowpath_fmt+0x30/0x40) [ 23.517021] [c001cabc] (warn_slowpath_fmt+0x30/0x40) from [c00d369c] (sysfs_remove_group+0x50/0x11c) [ 23.526504] [c00d369c] (sysfs_remove_group+0x50/0x11c) from [c0202ec4] (device_del+0x3c/0x1a4) [ 23.535437] [c0202ec4] (device_del+0x3c/0x1a4) from [c0295dfc] (sdio_remove_func+0x1c/0x2c) [ 23.544127] [c0295dfc] (sdio_remove_func+0x1c/0x2c) from [c0295110] (mmc_sdio_remove+0x44/0x78) [ 23.553161] [c0295110] (mmc_sdio_remove+0x44/0x78) from [c028f0e4] (mmc_stop_host+0xd0/0x22c) [ 23.562016] [c028f0e4] (mmc_stop_host+0xd0/0x22c) from [c0290578] (mmc_remove_host+0x18/0x2c) [ 23.570898] [c0290578] (mmc_remove_host+0x18/0x2c) from [bf019470] (if_sdio_reset_card_worker+0x18/0x2c [libertas_sdio]) [ 23.582138] [bf019470] (if_sdio_reset_card_worker+0x18/0x2c [libertas_sdio]) from [c00302fc] (process_one_work+0x260/0x3c8) [ 23.593594] [c00302fc] (process_one_work+0x260/0x3c8) from [c0030ac8] (worker_thread+0x224/0x458) [ 23.602816] [c0030ac8] (worker_thread+0x224/0x458) from [c0036a00] (kthread+0x80/0x88) [ 23.611078] [c0036a00] (kthread+0x80/0x88) from [c0009ca8] (kernel_thread_exit+0x0/0x8) [ 23.619409] ---[ end trace 78598ef84f325bdd ]--- [ 23.624011] Unable to handle kernel NULL pointer dereference at virtual address 000c [ 23.632079] pgd = c0004000 [ 23.634762] [000c] *pgd= [ 23.638351] Internal error: Oops: 17 [#1] PREEMPT [ 23.643016] Modules linked in: libertas_sdio libertas pxamci [ 23.648666] CPU: 0 Tainted: G W (3.2.0-rc2 #2) [ 23.654316] PC is at klist_put+0x18/0x9c [ 23.658213] LR is at device_del+0x50/0x1a4 [ 23.662287] pc : [c03ca8fc] lr : [c0202ed8] psr: a013 [ 23.662298] sp : c6569f18 ip : 4013 fp : c602e605 [ 23.673688] r10: r9 : bf019458 r8 : c602e600 [ 23.678876] r7 : r6 : c6447408 r5 : c64bbe50 r4 : 0001 [ 23.685363] r3 : r2 : 0200 r1 : 0001 r0 : 0001 [ 23.691844] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel [ 23.699106] Control: 397f Table: a65a0018 DAC: 0035 [ 23.704817] Process kworker/0:2 (pid: 1204, stack limit = 0xc6568278) [ 23.711218] Stack: (0xc6569f18 to 0xc656a000) [ 23.715547] 9f00: c65fe008 c6447000 [ 23.723677] 9f20: c6447408 c0202ed8 c65fe008 c6447000 0084 c0295dfc 0001 c0295110 [ 23.731810] 9f40: c63e8f00 c6447000 6013 0009 c028f0e4 c6447000 c6447000 [ 23.739944] 9f60: c63e8f00 c0290578 bf019c9c bf019470 bf019af0 c00302fc c05c1b38 c63e8f00 [ 23.748076] 9f80: c05c1b38 c05c1b38 0009 c6568000 c63e8f10 c05c1b38 c63e8f10 c0030ac8 [ 23.756208] 9fa0: c6083f14 c6569fd4 c6083f14 c63e8f00 c00308a4 [ 23.764341] 9fc0: c0036a00 c0009ca8 c63e8f00 c6569fd8 c6569fd8 [ 23.772475] 9fe0: c6083f14 c0036980 c0009ca8 0013 c0009ca8 0340f30e 153b5900 [ 23.780618] [c03ca8fc] (klist_put+0x18/0x9c) from [c0202ed8] (device_del+0x50/0x1a4) [ 23.788675] [c0202ed8] (device_del+0x50/0x1a4) from [c0295dfc] (sdio_remove_func+0x1c/0x2c) [ 23.797327] [c0295dfc] (sdio_remove_func+0x1c/0x2c) from [c0295110] (mmc_sdio_remove+0x44/0x78) [ 23.806328] [c0295110] (mmc_sdio_remove+0x44/0x78) from [c028f0e4] (mmc_stop_host+0xd0/0x22c) [ 23.815148] [c028f0e4] (mmc_stop_host+0xd0/0x22c) from [c0290578] (mmc_remove_host+0x18/0x2c) [ 23.823987] [c0290578] (mmc_remove_host+0x18/0x2c) from [bf019470] (if_sdio_reset_card_worker+0x18/0x2c [libertas_sdio]) [ 23.835164] [bf019470] (if_sdio_reset_card_worker+0x18/0x2c [libertas_sdio]) from [c00302fc] (process_one_work+0x260/0x3c8) [ 23.846584] [c00302fc] (process_one_work+0x260/0x3c8) from [c0030ac8] (worker_thread+0x224/0x458) [ 23.855760] [c0030ac8]
Re: Oops on suspend with libertas SDIO (Linux 3.2-rc2)
On Thu, Nov 17, 2011 at 8:19 PM, Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com wrote: [ Adding linux-pm mailing list to CC as well ] Thanks, Srivatsa S. Bhat On 11/17/2011 09:19 PM, Chris Ball wrote: Hi, On Thu, Nov 17 2011, Sven Neumann wrote: I've given 3.2-rc2 a try today and the kernel oopsed when going into suspend: [ 20.387912] Freezing user space processes ... (elapsed 0.01 seconds) done. [ 20.408759] Freezing remaining freezable tasks ... (elapsed 0.01 seconds) done. [ 20.432966] libertas_sdio mmc0:0001:1: mmc0:0001:1: suspend: PM flags = 0x0 [ 20.439978] libertas_sdio mmc0:0001:1: Suspend without wake params -- powering down card [ 23.456061] libertas_sdio mmc0:0001:1: wlan0: command 0x0010 timed out [ 23.462626] libertas_sdio mmc0:0001:1: wlan0: Timeout submitting command 0x0010 [ 23.470026] libertas_sdio: Resetting card... [ 23.474140] [ cut here ] [ 23.479002] WARNING: at fs/sysfs/group.c:138 sysfs_remove_group+0x50/0x11c() [ 23.486045] sysfs group c05cb5c8 not found for kobject 'mmc0:0001:1' [ 23.492358] Modules linked in: libertas_sdio libertas pxamci [ 23.498091] [c000dbb4] (unwind_backtrace+0x0/0xec) from [c001ca10] (warn_slowpath_common+0x4c/0x64) [ 23.507467] [c001ca10] (warn_slowpath_common+0x4c/0x64) from [c001cabc] (warn_slowpath_fmt+0x30/0x40) [ 23.517021] [c001cabc] (warn_slowpath_fmt+0x30/0x40) from [c00d369c] (sysfs_remove_group+0x50/0x11c) [ 23.526504] [c00d369c] (sysfs_remove_group+0x50/0x11c) from [c0202ec4] (device_del+0x3c/0x1a4) [ 23.535437] [c0202ec4] (device_del+0x3c/0x1a4) from [c0295dfc] (sdio_remove_func+0x1c/0x2c) [ 23.544127] [c0295dfc] (sdio_remove_func+0x1c/0x2c) from [c0295110] (mmc_sdio_remove+0x44/0x78) [ 23.553161] [c0295110] (mmc_sdio_remove+0x44/0x78) from [c028f0e4] (mmc_stop_host+0xd0/0x22c) [ 23.562016] [c028f0e4] (mmc_stop_host+0xd0/0x22c) from [c0290578] (mmc_remove_host+0x18/0x2c) [ 23.570898] [c0290578] (mmc_remove_host+0x18/0x2c) from [bf019470] (if_sdio_reset_card_worker+0x18/0x2c [libertas_sdio]) [ 23.582138] [bf019470] (if_sdio_reset_card_worker+0x18/0x2c [libertas_sdio]) from [c00302fc] (process_one_work+0x260/0x3c8) [ 23.593594] [c00302fc] (process_one_work+0x260/0x3c8) from [c0030ac8] (worker_thread+0x224/0x458) [ 23.602816] [c0030ac8] (worker_thread+0x224/0x458) from [c0036a00] (kthread+0x80/0x88) [ 23.611078] [c0036a00] (kthread+0x80/0x88) from [c0009ca8] (kernel_thread_exit+0x0/0x8) [ 23.619409] ---[ end trace 78598ef84f325bdd ]--- [ 23.624011] Unable to handle kernel NULL pointer dereference at virtual address 000c [ 23.632079] pgd = c0004000 [ 23.634762] [000c] *pgd= [ 23.638351] Internal error: Oops: 17 [#1] PREEMPT [ 23.643016] Modules linked in: libertas_sdio libertas pxamci [ 23.648666] CPU: 0 Tainted: G W (3.2.0-rc2 #2) [ 23.654316] PC is at klist_put+0x18/0x9c [ 23.658213] LR is at device_del+0x50/0x1a4 [ 23.662287] pc : [c03ca8fc] lr : [c0202ed8] psr: a013 [ 23.662298] sp : c6569f18 ip : 4013 fp : c602e605 [ 23.673688] r10: r9 : bf019458 r8 : c602e600 [ 23.678876] r7 : r6 : c6447408 r5 : c64bbe50 r4 : 0001 [ 23.685363] r3 : r2 : 0200 r1 : 0001 r0 : 0001 [ 23.691844] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel [ 23.699106] Control: 397f Table: a65a0018 DAC: 0035 [ 23.704817] Process kworker/0:2 (pid: 1204, stack limit = 0xc6568278) [ 23.711218] Stack: (0xc6569f18 to 0xc656a000) [ 23.715547] 9f00: c65fe008 c6447000 [ 23.723677] 9f20: c6447408 c0202ed8 c65fe008 c6447000 0084 c0295dfc 0001 c0295110 [ 23.731810] 9f40: c63e8f00 c6447000 6013 0009 c028f0e4 c6447000 c6447000 [ 23.739944] 9f60: c63e8f00 c0290578 bf019c9c bf019470 bf019af0 c00302fc c05c1b38 c63e8f00 [ 23.748076] 9f80: c05c1b38 c05c1b38 0009 c6568000 c63e8f10 c05c1b38 c63e8f10 c0030ac8 [ 23.756208] 9fa0: c6083f14 c6569fd4 c6083f14 c63e8f00 c00308a4 [ 23.764341] 9fc0: c0036a00 c0009ca8 c63e8f00 c6569fd8 c6569fd8 [ 23.772475] 9fe0: c6083f14 c0036980 c0009ca8 0013 c0009ca8 0340f30e 153b5900 [ 23.780618] [c03ca8fc] (klist_put+0x18/0x9c) from [c0202ed8] (device_del+0x50/0x1a4) [ 23.788675] [c0202ed8] (device_del+0x50/0x1a4) from [c0295dfc] (sdio_remove_func+0x1c/0x2c) [ 23.797327] [c0295dfc] (sdio_remove_func+0x1c/0x2c) from [c0295110] (mmc_sdio_remove+0x44/0x78) [ 23.806328] [c0295110] (mmc_sdio_remove+0x44/0x78) from [c028f0e4] (mmc_stop_host+0xd0/0x22c) [ 23.815148] [c028f0e4] (mmc_stop_host+0xd0/0x22c) from [c0290578] (mmc_remove_host+0x18/0x2c) [ 23.823987] [c0290578] (mmc_remove_host+0x18/0x2c) from [bf019470] (if_sdio_reset_card_worker+0x18/0x2c [libertas_sdio]) [ 23.835164] [bf019470] (if_sdio_reset_card_worker+0x18/0x2c [libertas_sdio]) from [c00302fc]
[PATCH] mmc: omap_hsmmc: fix typo around pr_info() call
This was introduced by a3c76eb9 (mmc: replace printk with appropriate display macro). Signed-off-by: Daniel Mack zon...@gmail.com Cc: Girish K S girish.shivananja...@linaro.org Cc: Chris Ball c...@laptop.org --- drivers/mmc/host/omap_hsmmc.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index e8ff123..101cd31 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1270,7 +1270,7 @@ static void omap_hsmmc_protect_card(struct omap_hsmmc_host *host) } } else { if (!host-protect_card) { - pr_info%s: cover is open, + pr_info(%s: cover is open, card is now inaccessible\n, mmc_hostname(host-mmc)); host-protect_card = 1; -- 1.7.6.4 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: irq flood with mmc boot partitions on s3c2416 with 3.0rc1
On Thu, Jun 2, 2011 at 9:49 PM, Heiko Stübner he...@sntech.de wrote: Hi, after upgrading my development kernel from 2.6.38 to 3.0rc1 I get flooded (i.e. it never stops) by messages of the form: Even though it's not really related to SDIO, does reverting 06e8935 (mmc: sdio: optimized SDIO IRQ handling for single irq) fix your problem? Daniel -- 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 v5] sdio: optimized SDIO IRQ handling for single irq
On Wed, May 11, 2011 at 6:45 PM, Chris Ball c...@laptop.org wrote: Hi Per, On Wed, May 11 2011, Per Forlin wrote: From: Stefan Nilsson XK stefan.xk.nils...@stericsson.com If there is only 1 function interrupt registered it is possible to improve performance by directly calling the irq handler and avoiding the overhead of reading the CCCR registers. Signed-off-by: Per Forlin per.for...@linaro.org Acked-by: Ulf Hansson ulf.hans...@stericsson.com Reviewed-by: Nicolas Pitre nicolas.pi...@linaro.org --- drivers/mmc/core/sdio_irq.c | 33 - include/linux/mmc/card.h | 1 + 2 files changed, 33 insertions(+), 1 deletions(-) diff --git a/drivers/mmc/core/sdio_irq.c b/drivers/mmc/core/sdio_irq.c index bb192f9..a0890ac 100644 --- a/drivers/mmc/core/sdio_irq.c +++ b/drivers/mmc/core/sdio_irq.c @@ -31,6 +31,17 @@ static int process_sdio_pending_irqs(struct mmc_card *card) { int i, ret, count; unsigned char pending; + struct sdio_func *func; + + /* + * Optimization, if there is only 1 function interrupt registered + * call irq handler directly + */ + func = card-sdio_single_irq; + if (func) { + func-irq_handler(func); + return 1; + } ret = mmc_io_rw_direct(card, 0, 0, SDIO_CCCR_INTx, 0, pending); if (ret) { @@ -42,7 +53,7 @@ static int process_sdio_pending_irqs(struct mmc_card *card) count = 0; for (i = 1; i = 7; i++) { if (pending (1 i)) { - struct sdio_func *func = card-sdio_func[i - 1]; + func = card-sdio_func[i - 1]; if (!func) { printk(KERN_WARNING %s: pending IRQ for non-existant function\n, @@ -186,6 +197,24 @@ static int sdio_card_irq_put(struct mmc_card *card) return 0; } +/* If there is only 1 function registered set sdio_single_irq */ +static void sdio_single_irq_set(struct mmc_card *card) +{ + struct sdio_func *func; + int i; + + card-sdio_single_irq = NULL; + if ((card-host-caps MMC_CAP_SDIO_IRQ) + card-host-sdio_irqs == 1) + for (i = 0; i card-sdio_funcs; i++) { + func = card-sdio_func[i]; + if (func func-irq_handler) { + card-sdio_single_irq = func; + break; + } + } +} + /** * sdio_claim_irq - claim the IRQ for a SDIO function * @func: SDIO function @@ -227,6 +256,7 @@ int sdio_claim_irq(struct sdio_func *func, sdio_irq_handler_t *handler) ret = sdio_card_irq_get(func-card); if (ret) func-irq_handler = NULL; + sdio_single_irq_set(func-card); return ret; } @@ -251,6 +281,7 @@ int sdio_release_irq(struct sdio_func *func) if (func-irq_handler) { func-irq_handler = NULL; sdio_card_irq_put(func-card); + sdio_single_irq_set(func-card); } ret = mmc_io_rw_direct(func-card, 0, 0, SDIO_CCCR_IENx, 0, reg); diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index d8dffc9..4910dec 100644 --- a/include/linux/mmc/card.h +++ b/include/linux/mmc/card.h @@ -191,6 +191,7 @@ struct mmc_card { struct sdio_cccr cccr; /* common card info */ struct sdio_cis cis; /* common tuple info */ struct sdio_func *sdio_func[SDIO_MAX_FUNCS]; /* SDIO functions (devices) */ + struct sdio_func *sdio_single_irq; /* SDIO function when only one IRQ active */ unsigned num_info; /* number of info strings */ const char **info; /* info strings */ struct sdio_func_tuple *tuples; /* unknown common tuples */ Thanks, looks good now -- pushed to mmc-next for .40. This patch breaks libertas over SDIO, as the interrupt handler of that driver is now called even though the hardware didn't signal any interrupt condition. This is a problem for at least two reasons in this case: a) the libertas code is currently structured in a way that it calls sdio_claim_irq() before everything is fully set up (which wasn't a problem so far as the hardware has an own register to mask interrupts). That results in a kernel Ooops as card-priv == NULL. b) the libertas interrupt handler assumes that a received callback to its interrupt handler signals activity per se (card-priv-activty_detected = 1). I have a patch ready for libertas that works around the first problem, but I'm unsure if that's not fixing the wrong end. What the driver really should do is poll the card's CCCR register to find out if there was any interrupt at all and bail if that's not the case, but then we're back to the implementation without this patch. The other option is to revert the patch again.
Re: [PATCH v5] sdio: optimized SDIO IRQ handling for single irq
On Mon, Jun 6, 2011 at 3:17 PM, Daniel Mack zon...@gmail.com wrote: On Wed, May 11, 2011 at 6:45 PM, Chris Ball c...@laptop.org wrote: Hi Per, On Wed, May 11 2011, Per Forlin wrote: From: Stefan Nilsson XK stefan.xk.nils...@stericsson.com If there is only 1 function interrupt registered it is possible to improve performance by directly calling the irq handler and avoiding the overhead of reading the CCCR registers. Signed-off-by: Per Forlin per.for...@linaro.org Acked-by: Ulf Hansson ulf.hans...@stericsson.com Reviewed-by: Nicolas Pitre nicolas.pi...@linaro.org --- drivers/mmc/core/sdio_irq.c | 33 - include/linux/mmc/card.h | 1 + 2 files changed, 33 insertions(+), 1 deletions(-) diff --git a/drivers/mmc/core/sdio_irq.c b/drivers/mmc/core/sdio_irq.c index bb192f9..a0890ac 100644 --- a/drivers/mmc/core/sdio_irq.c +++ b/drivers/mmc/core/sdio_irq.c @@ -31,6 +31,17 @@ static int process_sdio_pending_irqs(struct mmc_card *card) { int i, ret, count; unsigned char pending; + struct sdio_func *func; + + /* + * Optimization, if there is only 1 function interrupt registered + * call irq handler directly + */ + func = card-sdio_single_irq; + if (func) { + func-irq_handler(func); + return 1; + } ret = mmc_io_rw_direct(card, 0, 0, SDIO_CCCR_INTx, 0, pending); if (ret) { @@ -42,7 +53,7 @@ static int process_sdio_pending_irqs(struct mmc_card *card) count = 0; for (i = 1; i = 7; i++) { if (pending (1 i)) { - struct sdio_func *func = card-sdio_func[i - 1]; + func = card-sdio_func[i - 1]; if (!func) { printk(KERN_WARNING %s: pending IRQ for non-existant function\n, @@ -186,6 +197,24 @@ static int sdio_card_irq_put(struct mmc_card *card) return 0; } +/* If there is only 1 function registered set sdio_single_irq */ +static void sdio_single_irq_set(struct mmc_card *card) +{ + struct sdio_func *func; + int i; + + card-sdio_single_irq = NULL; + if ((card-host-caps MMC_CAP_SDIO_IRQ) + card-host-sdio_irqs == 1) + for (i = 0; i card-sdio_funcs; i++) { + func = card-sdio_func[i]; + if (func func-irq_handler) { + card-sdio_single_irq = func; + break; + } + } +} + /** * sdio_claim_irq - claim the IRQ for a SDIO function * @func: SDIO function @@ -227,6 +256,7 @@ int sdio_claim_irq(struct sdio_func *func, sdio_irq_handler_t *handler) ret = sdio_card_irq_get(func-card); if (ret) func-irq_handler = NULL; + sdio_single_irq_set(func-card); return ret; } @@ -251,6 +281,7 @@ int sdio_release_irq(struct sdio_func *func) if (func-irq_handler) { func-irq_handler = NULL; sdio_card_irq_put(func-card); + sdio_single_irq_set(func-card); } ret = mmc_io_rw_direct(func-card, 0, 0, SDIO_CCCR_IENx, 0, reg); diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index d8dffc9..4910dec 100644 --- a/include/linux/mmc/card.h +++ b/include/linux/mmc/card.h @@ -191,6 +191,7 @@ struct mmc_card { struct sdio_cccr cccr; /* common card info */ struct sdio_cis cis; /* common tuple info */ struct sdio_func *sdio_func[SDIO_MAX_FUNCS]; /* SDIO functions (devices) */ + struct sdio_func *sdio_single_irq; /* SDIO function when only one IRQ active */ unsigned num_info; /* number of info strings */ const char **info; /* info strings */ struct sdio_func_tuple *tuples; /* unknown common tuples */ Thanks, looks good now -- pushed to mmc-next for .40. This patch breaks libertas over SDIO, as the interrupt handler of that driver is now called even though the hardware didn't signal any interrupt condition. This is a problem for at least two reasons in this case: I checked the libertas-dev archives too late and saw that there is a fix by Daniel Drake for this problem already. Sorry for the noise. Daniel -- 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: 2.6.35.6 fails to suspend (pxa2xx-mci.0)
On Mon, Oct 04, 2010 at 09:30:35AM +0200, Sven Neumann wrote: we are running an embedded system here based on the PXA300 platform. Suspend/resume used to work well so far. However after upgrading the kernel from 2.6.34.7 to 2.6.35.6, we get the following error when trying to suspend the system: # echo mem /sys/power/state [ 5647.295953] PM: Syncing filesystems ... done. [ 5647.318792] Freezing user space processes ... (elapsed 0.01 seconds) done. [ 5647.337048] Freezing remaining freezable tasks ... (elapsed 0.01 seconds) done. [ 5647.356915] Suspending console(s) (use no_console_suspend to debug) [ 5647.366651] pm_op(): platform_pm_suspend+0x0/0x5c returns -38 [ 5647.366671] PM: Device pxa2xx-mci.0 failed to suspend: error -38 [ 5647.367082] PM: Some devices failed to suspend We've bisected this effect down to commit 152e1d5920 (PM: Prevent waiting forever on asynchronous resume after failing suspend). Suspending our PXA3xx based system breaks with this patch. I tried to understand what's going wrong, but I didn't follow the discussion about this logic, so I would rather like to pass it back to the originating people. I can only guess that the problem here is the somewhat tricky handling around mmc_sdio_suspend(), which returns -ENODEV (-38) in case a particular function of a card can not be suspended. The SDIO core would have simply removed the card in this case normally, but the PM core seems to interfere now, stopping the whole suspend procedure. Can anyone shed some light on this? Thanks, Daniel -- 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
Libertas/SDIO deep sleep mode
Hi, I read about the recently added features in the libertas driver to send the chip to its deep-sleep mode when the system suspends, so the wakeup phase is shortened. I tried this with a recent 2.6.35-rc1 kernel on a PXA300 based design, and this is what I get: [0.721643] Freezing user space processes ... (elapsed 0.01 seconds) done. [0.740588] Freezing remaining freezable tasks ... (elapsed 0.01 seconds) done. [0.760687] Suspending console(s) (use no_console_suspend to debug) [0.879657] libertas: 00:19:88:3a:cb:ac, fw 9.70.3p36, cap 0x0303 [0.880567] [ cut here ] [0.880622] WARNING: at drivers/base/power/main.c:101 device_pm_add+0x94/0xd8() [0.880639] Device: ieee80211 [0.880646] Parentless device registered during a PM transaction (phy1) [0.880660] Modules linked in: eeti_ts libertas_sdio libertas pxamci ds2760_battery w1_ds2760 wire [0.880754] [c00493a0] (unwind_backtrace+0x0/0xec) from [c0056db0] (warn_slowpath_common+0x4c/0x7c) [0.880793] [c0056db0] (warn_slowpath_common+0x4c/0x7c) from [c0056e74] (warn_slowpath_fmt+0x30/0x40) [0.880829] [c0056e74] (warn_slowpath_fmt+0x30/0x40) from [c01d4cc8] (device_pm_add+0x94/0xd8) [0.880875] [c01d4cc8] (device_pm_add+0x94/0xd8) from [c01ce4dc] (device_add+0x350/0x510) [0.880910] [c01ce4dc] (device_add+0x350/0x510) from [c032d500] (wiphy_register+0x1ac/0x298) [0.881033] [c032d500] (wiphy_register+0x1ac/0x298) from [bf0285dc] (lbs_cfg_register+0x54/0x9c [libertas]) [0.881142] [bf0285dc] (lbs_cfg_register+0x54/0x9c [libertas]) from [bf02c228] (lbs_start_card+0xa0/0x130 [libertas]) [0.881226] [bf02c228] (lbs_start_card+0xa0/0x130 [libertas]) from [bf040b94] (if_sdio_probe+0x84c/0x954 [libertas_sdio]) [0.881276] [bf040b94] (if_sdio_probe+0x84c/0x954 [libertas_sdio]) from [c024fd98] (sdio_bus_probe+0xcc/0xe8) [0.881322] [c024fd98] (sdio_bus_probe+0xcc/0xe8) from [c01d069c] (driver_probe_device+0xb4/0x198) [0.881359] [c01d069c] (driver_probe_device+0xb4/0x198) from [c01cfc08] (bus_for_each_drv+0x4c/0x8c) [0.881396] [c01cfc08] (bus_for_each_drv+0x4c/0x8c) from [c01d08d4] (device_attach+0x54/0x70) [0.881430] [c01d08d4] (device_attach+0x54/0x70) from [c01cfa2c] (bus_probe_device+0x28/0x50) [0.881464] [c01cfa2c] (bus_probe_device+0x28/0x50) from [c01ce510] (device_add+0x384/0x510) [0.881496] [c01ce510] (device_add+0x384/0x510) from [c024fbd4] (sdio_add_func+0x38/0x54) [0.881527] [c024fbd4] (sdio_add_func+0x38/0x54) from [c024f28c] (mmc_attach_sdio+0x21c/0x2a8) [0.881581] [c024f28c] (mmc_attach_sdio+0x21c/0x2a8) from [c024b97c] (mmc_rescan+0x1ec/0x28c) [0.881624] [c024b97c] (mmc_rescan+0x1ec/0x28c) from [c0067a60] (worker_thread+0x168/0x1f0) [0.881667] [c0067a60] (worker_thread+0x168/0x1f0) from [c006abd8] (kthread+0x78/0x80) [0.881720] [c006abd8] (kthread+0x78/0x80) from [c00458bc] (kernel_thread_exit+0x0/0x8) [0.881742] ---[ end trace 919a8d4b06013c07 ]--- [0.884319] libertas: wlan0: Marvell WLAN 802.11 adapter [1.138756] mmc0: card 0001 removed [1.139448] PM: suspend of devices complete after 255.035 msecs [1.140076] PM: late suspend of devices complete after 0.598 msecs [0.000943] PM: early resume of devices complete after 0.634 msecs [0.060955] usb usb1: root hub lost power or was reset [0.136987] mmc0: new SDIO card at address 0001 [0.137101] mmc mmc0:0001: parent mmc0 should not be sleeping [0.298199] PM: resume of devices complete after 297.118 msecs [0.571350] Restarting tasks ... [0.768454] done. [1.179764] libertas: 00:19:88:3a:cb:ac, fw 9.70.3p36, cap 0x0303 [1.198354] libertas: wlan0: Marvell WLAN 802.11 adapter Also, the 8686's supply voltage (which is controlled via a regulator in the pxamci code) drops to zero during the suspend phase. Here are some questions that I have about this: * The Parentless device registered during a PM transaction looks like a bug to me. Note that I added kobject_name(dev-kobj) to the output to get a clue about what's going on. * The firmware survives the suspend/resume transition and works fine after that. I just wonder why, as the voltage was switched off? * I believe that if we need to keep the power switched on, there should be a call to sdio_set_host_pm_flags(MMC_PM_KEEP_POWER) somewhere in the libertas_sdio layer, right? How should the pxamci core get to know it shouldn't switch off its regulator? * Should the message about mmc0 which should not be sleeping go away once the other issues are solved? Thanks, Daniel -- 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] libertas/sdio: 8686: set ECSI bit for 1-bit transfers
On Tue, Apr 06, 2010 at 09:07:34AM -0700, Dan Williams wrote: On Tue, 2010-04-06 at 10:52 +0200, Daniel Mack wrote: When operating in 1-bit mode, SDAT1 is used as dedicated interrupt line. However, the 8686 will only drive this line when the ECSI bit is set in the CCCR_IF register. Thanks to Alagu Sankar for pointing me in the right direction. Signed-off-by: Daniel Mack dan...@caiaq.de Cc: Alagu Sankar alagusan...@embwise.com Cc: Volker Ernst volker.er...@txtr.com Cc: Dan Williams d...@redhat.com Cc: John W. Linville linvi...@tuxdriver.com Cc: Holger Schurig hs4...@mail.mn-solutions.de Cc: Bing Zhao bz...@marvell.com Cc: libertas-...@lists.infradead.org Cc: linux-wirel...@vger.kernel.org Cc: linux-mmc@vger.kernel.org Acked-by: Dan Williams d...@redhat.com Was this picked by anyone? Just asking because I didn't see it in the wireless-2.6.git yet. Thanks, Daniel --- drivers/net/wireless/libertas/if_sdio.c | 22 ++ include/linux/mmc/sdio.h|2 ++ 2 files changed, 24 insertions(+), 0 deletions(-) diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c index 7a73f62..33206a9 100644 --- a/drivers/net/wireless/libertas/if_sdio.c +++ b/drivers/net/wireless/libertas/if_sdio.c @@ -34,6 +34,8 @@ #include linux/mmc/card.h #include linux/mmc/sdio_func.h #include linux/mmc/sdio_ids.h +#include linux/mmc/sdio.h +#include linux/mmc/host.h #include host.h #include decl.h @@ -942,6 +944,7 @@ static int if_sdio_probe(struct sdio_func *func, int ret, i; unsigned int model; struct if_sdio_packet *packet; + struct mmc_host *host = func-card-host; lbs_deb_enter(LBS_DEB_SDIO); @@ -1022,6 +1025,25 @@ static int if_sdio_probe(struct sdio_func *func, if (ret) goto disable; + /* For 1-bit transfers to the 8686 model, we need to enable the +* interrupt flag in the CCCR register. Set the MMC_QUIRK_LENIENT_FN0 +* bit to allow access to non-vendor registers. */ + if ((card-model == IF_SDIO_MODEL_8686) + (host-caps MMC_CAP_SDIO_IRQ) + (host-ios.bus_width == MMC_BUS_WIDTH_1)) { + u8 reg; + + func-card-quirks |= MMC_QUIRK_LENIENT_FN0; + reg = sdio_f0_readb(func, SDIO_CCCR_IF, ret); + if (ret) + goto release_int; + + reg |= SDIO_BUS_ECSI; + sdio_f0_writeb(func, reg, SDIO_CCCR_IF, ret); + if (ret) + goto release_int; + } + card-ioport = sdio_readb(func, IF_SDIO_IOPORT, ret); if (ret) goto release_int; diff --git a/include/linux/mmc/sdio.h b/include/linux/mmc/sdio.h index 0ebaef5..329a8fa 100644 --- a/include/linux/mmc/sdio.h +++ b/include/linux/mmc/sdio.h @@ -94,6 +94,8 @@ #define SDIO_BUS_WIDTH_1BIT 0x00 #define SDIO_BUS_WIDTH_4BIT 0x02 +#define SDIO_BUS_ECSI 0x20/* Enable continuous SPI interrupt */ +#define SDIO_BUS_SCSI 0x40/* Support continuous SPI interrupt */ #define SDIO_BUS_ASYNC_INT0x20 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] ARM: MXC: mxcmmc: misc cleanups
On Wed, Mar 31, 2010 at 02:38:56PM +0200, Sascha Hauer wrote: On Tue, Mar 30, 2010 at 08:31:59PM +0200, Daniel Mack wrote: + dev_err(mmc_dev(host-mmc), + %s: read -ETIMEDOUT\n, __func__); data-error = -ETIMEDOUT; } else { + dev_err(mmc_dev(host-mmc), %s: -EIO\n, __func__); Do we really want to have these messages with dev_err? In the subject you are talking about debug output. This _is_ definitely an error if get there, so I'll rather change the commit message ;) data-error = -EIO; } - } else { + } else data-bytes_xfered = host-datasize; - } Documentation/CodingStyle says that if braces are used in one branch of a conditional then they should be used in both branches. I personally don't care much about this statement but I think we should leave it as is. Otherwise some day someone wants to change it back according to the coding style. Ok, true. I'll fix this (and the commit message) and resend. Thanks, Daniel -- 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] libertas/sdio: set ECSI and SCSI bits for 1-bit transfers
On Wed, Mar 31, 2010 at 11:49:49AM +0200, Michał Mirosław wrote: W dniu 31 marca 2010 11:08 użytkownik Daniel Mack dan...@caiaq.de napisał: Hmm, that function isn't exported, and I didn't want to change this. You say you'd prefer that? I can cook up something that does it, no problem. BTW, I can't see any exported functions to access fn#0 directly from drivers. Maybe it's time to introduce them now - at least CCCR has some vendor-defined parts that drivers may want to access and there is a lot of place in CIS area that can be (ab)used by devices. In fact, there is sdio_f0_{read,write}b() - I overlooked them. Will resend a new patch. Thanks for checking, Daniel -- 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] ARM: MXC: mxcmmc: Teach the driver SDIO operations
On Wed, Mar 31, 2010 at 03:03:39PM +0200, Sascha Hauer wrote: On Tue, Mar 30, 2010 at 08:32:00PM +0200, Daniel Mack wrote: +static void mxcmci_enable_sdio_irq(struct mmc_host *mmc, int enable) +{ + struct mxcmci_host *host = mmc_priv(mmc); + unsigned long flags; + u32 int_cntr; + + spin_lock_irqsave(host-lock, flags); + host-use_sdio = enable; + int_cntr = readl(host-base + MMC_REG_INT_CNTR); + + if (enable) + int_cntr |= INT_SDIO_IRQ_EN; + else + int_cntr = ~INT_SDIO_IRQ_EN; + + writel(int_cntr, host-base + MMC_REG_INT_CNTR); + spin_unlock_irqrestore(host-lock, flags); The other places where MMC_REG_INT_CNTR is touched should be protected by this spinlock aswell. Hmm, all other place don't do a read/modify/write cycle, so I'd say the don't need protection? Daniel -- 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] libertas/sdio: set ECSI bit for 1-bit transfers
When operating in 1-bit mode, SDAT1 is used as dedicated interrupt line. However, the 8686 will only drive this line when the ECSI bit is set in the CCCR_IF register. Thanks to Alagu Sankar for pointing me in the right direction. Signed-off-by: Daniel Mack dan...@caiaq.de Cc: Alagu Sankar alagusan...@embwise.com Cc: Volker Ernst volker.er...@txtr.com Cc: Dan Williams d...@redhat.com Cc: John W. Linville linvi...@tuxdriver.com Cc: Holger Schurig hs4...@mail.mn-solutions.de Cc: Bing Zhao bz...@marvell.com Cc: libertas-...@lists.infradead.org Cc: linux-wirel...@vger.kernel.org Cc: linux-mmc@vger.kernel.org --- drivers/net/wireless/libertas/if_sdio.c | 21 + include/linux/mmc/sdio.h|2 ++ 2 files changed, 23 insertions(+), 0 deletions(-) diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c index 7a73f62..f89bb8b 100644 --- a/drivers/net/wireless/libertas/if_sdio.c +++ b/drivers/net/wireless/libertas/if_sdio.c @@ -34,6 +34,8 @@ #include linux/mmc/card.h #include linux/mmc/sdio_func.h #include linux/mmc/sdio_ids.h +#include linux/mmc/sdio.h +#include linux/mmc/host.h #include host.h #include decl.h @@ -942,6 +944,7 @@ static int if_sdio_probe(struct sdio_func *func, int ret, i; unsigned int model; struct if_sdio_packet *packet; + struct mmc_host *host = func-card-host; lbs_deb_enter(LBS_DEB_SDIO); @@ -1022,6 +1025,24 @@ static int if_sdio_probe(struct sdio_func *func, if (ret) goto disable; + /* For 1-bit transfers, we need to enable the interrupt flag in +* the CCCR register. Set the MMC_QUIRK_LENIENT_FN0 bit to allow +* access to non-vendor registers. */ + if ((host-caps MMC_CAP_SDIO_IRQ) + (host-ios.bus_width == MMC_BUS_WIDTH_1)) { + u8 reg; + + func-card-quirks |= MMC_QUIRK_LENIENT_FN0; + reg = sdio_f0_readb(func, SDIO_CCCR_IF, ret); + if (ret) + goto release_int; + + reg |= SDIO_BUS_ECSI; + sdio_f0_writeb(func, reg, SDIO_CCCR_IF, ret); + if (ret) + goto release_int; + } + card-ioport = sdio_readb(func, IF_SDIO_IOPORT, ret); if (ret) goto release_int; diff --git a/include/linux/mmc/sdio.h b/include/linux/mmc/sdio.h index 0ebaef5..329a8fa 100644 --- a/include/linux/mmc/sdio.h +++ b/include/linux/mmc/sdio.h @@ -94,6 +94,8 @@ #define SDIO_BUS_WIDTH_1BIT 0x00 #define SDIO_BUS_WIDTH_4BIT 0x02 +#define SDIO_BUS_ECSI 0x20/* Enable continuous SPI interrupt */ +#define SDIO_BUS_SCSI 0x40/* Support continuous SPI interrupt */ #define SDIO_BUS_ASYNC_INT0x20 -- 1.7.0 -- 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] libertas/sdio: set ECSI and SCSI bits for 1-bit transfers
On Wed, Mar 31, 2010 at 01:53:21PM +0530, Alagu Sankar Vellaichamy wrote: SCSI is read-only. We ideally should be checking this bit and then set the ECSI accordingly, rather than setting both ECSI and SCSI. Thanks for noticing. However, the libertas chip does not set SCSI, so we have to do it unconditionally. New patch has just been sent. Daniel On Tue, Mar 30, 2010 at 11:08 PM, Daniel Mack dan...@caiaq.de wrote: When operating in 1-bit mode, SDAT1 is used as dedicated interrupt line. However, the 8686 will only drive this line when the ECSI and SCSI bits are set in the CCCR_IF register. Thanks to Alagu Sankar for pointing me in the right direction. Signed-off-by: Daniel Mack dan...@caiaq.de Cc: Alagu Sankar alagusan...@embwise.com Cc: Volker Ernst volker.er...@txtr.com Cc: Dan Williams d...@redhat.com Cc: John W. Linville linvi...@tuxdriver.com Cc: Holger Schurig hs4...@mail.mn-solutions.de Cc: Bing Zhao bz...@marvell.com Cc: libertas-...@lists.infradead.org Cc: linux-wirel...@vger.kernel.org Cc: linux-mmc@vger.kernel.org --- drivers/net/wireless/libertas/if_sdio.c | 24 include/linux/mmc/sdio.h | 2 ++ 2 files changed, 26 insertions(+), 0 deletions(-) diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c index 7a73f62..d3170f2 100644 --- a/drivers/net/wireless/libertas/if_sdio.c +++ b/drivers/net/wireless/libertas/if_sdio.c @@ -34,6 +34,8 @@ #include linux/mmc/card.h #include linux/mmc/sdio_func.h #include linux/mmc/sdio_ids.h +#include linux/mmc/sdio.h +#include linux/mmc/host.h #include host.h #include decl.h @@ -942,6 +944,7 @@ static int if_sdio_probe(struct sdio_func *func, int ret, i; unsigned int model; struct if_sdio_packet *packet; + struct mmc_host *host = func-card-host; lbs_deb_enter(LBS_DEB_SDIO); @@ -1022,6 +1025,27 @@ static int if_sdio_probe(struct sdio_func *func, if (ret) goto disable; + /* For 1-bit transfers, we need to enable the interrupt flags in + * the CCCR register. Temporarily set the function number to 0 + * for that. */ + if ((host-caps MMC_CAP_SDIO_IRQ) + (host-ios.bus_width == MMC_BUS_WIDTH_1)) { + unsigned int num = func-num; + u8 reg; + + func-num = 0; + reg = sdio_readb(func, SDIO_CCCR_IF, ret); + if (ret) + goto release_int; + + reg |= SDIO_BUS_ECSI | SDIO_BUS_SCSI; + sdio_writeb(func, reg, SDIO_CCCR_IF, ret); + if (ret) + goto release_int; + + func-num = num; + } + card-ioport = sdio_readb(func, IF_SDIO_IOPORT, ret); if (ret) goto release_int; diff --git a/include/linux/mmc/sdio.h b/include/linux/mmc/sdio.h index 0ebaef5..329a8fa 100644 --- a/include/linux/mmc/sdio.h +++ b/include/linux/mmc/sdio.h @@ -94,6 +94,8 @@ #define SDIO_BUS_WIDTH_1BIT 0x00 #define SDIO_BUS_WIDTH_4BIT 0x02 +#define SDIO_BUS_ECSI 0x20 /* Enable continuous SPI interrupt */ +#define SDIO_BUS_SCSI 0x40 /* Support continuous SPI interrupt */ #define SDIO_BUS_ASYNC_INT 0x20 -- 1.7.0 -- 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] ARM: MXC: mxcmmc: Teach the driver SDIO operations
Successfully tested on MX31 hardware using libertas SDIO peripherals. Signed-off-by: Daniel Mack dan...@caiaq.de Cc: Sascha Hauer s.ha...@pengutronix.de Cc: Dan Williams dan.j.willi...@intel.com Cc: Volker Ernst volker.er...@txtr.com Cc: Jiri Kosina jkos...@suse.cz --- drivers/mmc/host/mxcmmc.c | 55 1 files changed, 45 insertions(+), 10 deletions(-) diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c index 21037ff..83a4545 100644 --- a/drivers/mmc/host/mxcmmc.c +++ b/drivers/mmc/host/mxcmmc.c @@ -119,6 +119,7 @@ struct mxcmci_host { int detect_irq; int dma; int do_dma; + int use_sdio; unsigned intpower_mode; struct imxmmc_platform_data *pdata; @@ -138,6 +139,7 @@ struct mxcmci_host { int clock; struct work_struct datawork; + spinlock_t lock; }; static void mxcmci_set_clk_rate(struct mxcmci_host *host, unsigned int clk_ios); @@ -226,6 +228,8 @@ static int mxcmci_setup_data(struct mxcmci_host *host, struct mmc_data *data) static int mxcmci_start_cmd(struct mxcmci_host *host, struct mmc_command *cmd, unsigned int cmdat) { + u32 int_cntr; + WARN_ON(host-cmd != NULL); host-cmd = cmd; @@ -249,13 +253,15 @@ static int mxcmci_start_cmd(struct mxcmci_host *host, struct mmc_command *cmd, return -EINVAL; } + int_cntr = INT_END_CMD_RES_EN; + if (mxcmci_use_dma(host)) - writel(INT_READ_OP_EN | INT_WRITE_OP_DONE_EN | - INT_END_CMD_RES_EN, - host-base + MMC_REG_INT_CNTR); - else - writel(INT_END_CMD_RES_EN, host-base + MMC_REG_INT_CNTR); + int_cntr |= INT_READ_OP_EN | INT_WRITE_OP_DONE_EN; + + if (host-use_sdio) + int_cntr |= INT_SDIO_IRQ_EN; + writel(int_cntr, host-base + MMC_REG_INT_CNTR); writew(cmd-opcode, host-base + MMC_REG_CMD); writel(cmd-arg, host-base + MMC_REG_ARG); writew(cmdat, host-base + MMC_REG_CMD_DAT_CONT); @@ -266,7 +272,12 @@ static int mxcmci_start_cmd(struct mxcmci_host *host, struct mmc_command *cmd, static void mxcmci_finish_request(struct mxcmci_host *host, struct mmc_request *req) { - writel(0, host-base + MMC_REG_INT_CNTR); + u32 int_cntr = 0; + + if (host-use_sdio) + int_cntr |= INT_SDIO_IRQ_EN; + + writel(int_cntr, host-base + MMC_REG_INT_CNTR); host-req = NULL; host-cmd = NULL; @@ -536,8 +547,12 @@ static irqreturn_t mxcmci_irq(int irq, void *devid) dev_dbg(mmc_dev(host-mmc), %s: 0x%08x\n, __func__, stat); + if ((stat STATUS_SDIO_INT_ACTIVE) host-use_sdio) + mmc_signal_sdio_irq(host-mmc); + if (stat STATUS_END_CMD_RESP) mxcmci_cmd_done(host, stat); + #ifdef HAS_DMA if (mxcmci_use_dma(host) (stat (STATUS_DATA_TRANS_DONE | STATUS_WRITE_OP_DONE))) @@ -674,11 +689,30 @@ static int mxcmci_get_ro(struct mmc_host *mmc) return -ENOSYS; } +static void mxcmci_enable_sdio_irq(struct mmc_host *mmc, int enable) +{ + struct mxcmci_host *host = mmc_priv(mmc); + unsigned long flags; + u32 int_cntr; + + spin_lock_irqsave(host-lock, flags); + host-use_sdio = enable; + int_cntr = readl(host-base + MMC_REG_INT_CNTR); + + if (enable) + int_cntr |= INT_SDIO_IRQ_EN; + else + int_cntr = ~INT_SDIO_IRQ_EN; + + writel(int_cntr, host-base + MMC_REG_INT_CNTR); + spin_unlock_irqrestore(host-lock, flags); +} static const struct mmc_host_ops mxcmci_ops = { - .request= mxcmci_request, - .set_ios= mxcmci_set_ios, - .get_ro = mxcmci_get_ro, + .request= mxcmci_request, + .set_ios= mxcmci_set_ios, + .get_ro = mxcmci_get_ro, + .enable_sdio_irq= mxcmci_enable_sdio_irq, }; static int mxcmci_probe(struct platform_device *pdev) @@ -706,7 +740,7 @@ static int mxcmci_probe(struct platform_device *pdev) } mmc-ops = mxcmci_ops; - mmc-caps = MMC_CAP_4_BIT_DATA; + mmc-caps = MMC_CAP_4_BIT_DATA | MMC_CAP_SDIO_IRQ; /* MMC core transfer sizes tunable parameters */ mmc-max_hw_segs = 64; @@ -725,6 +759,7 @@ static int mxcmci_probe(struct platform_device *pdev) host-mmc = mmc; host-pdata = pdev-dev.platform_data; + spin_lock_init(host-lock); if (host-pdata host-pdata-ocr_avail) mmc-ocr_avail = host-pdata-ocr_avail; -- 1.7.0 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More
[PATCH 1/3] ARM: MXC: mxcmmc: misc cleanups
Add some more debug information and fix a couple of coding style things in mxcmmc.c. Signed-off-by: Daniel Mack dan...@caiaq.de Cc: Sascha Hauer s.ha...@pengutronix.de Cc: Dan Williams dan.j.willi...@intel.com Cc: Volker Ernst volker.er...@txtr.com Cc: Jiri Kosina jkos...@suse.cz --- drivers/mmc/host/mxcmmc.c | 28 +--- 1 files changed, 17 insertions(+), 11 deletions(-) diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c index 2df9041..21037ff 100644 --- a/drivers/mmc/host/mxcmmc.c +++ b/drivers/mmc/host/mxcmmc.c @@ -151,6 +151,8 @@ static void mxcmci_softreset(struct mxcmci_host *host) { int i; + dev_dbg(mmc_dev(host-mmc), mxcmci_softreset\n); + /* reset sequence */ writew(STR_STP_CLK_RESET, host-base + MMC_REG_STR_STP_CLK); writew(STR_STP_CLK_RESET | STR_STP_CLK_START_CLK, @@ -290,21 +292,29 @@ static int mxcmci_finish_data(struct mxcmci_host *host, unsigned int stat) dev_dbg(mmc_dev(host-mmc), request failed. status: 0x%08x\n, stat); if (stat STATUS_CRC_READ_ERR) { + dev_err(mmc_dev(host-mmc), %s: -EILSEQ\n, __func__); data-error = -EILSEQ; } else if (stat STATUS_CRC_WRITE_ERR) { u32 err_code = (stat 9) 0x3; - if (err_code == 2) /* No CRC response */ + if (err_code == 2) { /* No CRC response */ + dev_err(mmc_dev(host-mmc), + %s: No CRC -ETIMEDOUT\n, __func__); data-error = -ETIMEDOUT; - else + } else { + dev_err(mmc_dev(host-mmc), + %s: -EILSEQ\n, __func__); data-error = -EILSEQ; + } } else if (stat STATUS_TIME_OUT_READ) { + dev_err(mmc_dev(host-mmc), + %s: read -ETIMEDOUT\n, __func__); data-error = -ETIMEDOUT; } else { + dev_err(mmc_dev(host-mmc), %s: -EIO\n, __func__); data-error = -EIO; } - } else { + } else data-bytes_xfered = host-datasize; - } data_error = data-error; @@ -433,8 +443,6 @@ static int mxcmci_transfer_data(struct mxcmci_host *host) struct scatterlist *sg; int stat, i; - host-datasize = 0; - host-data = data; host-datasize = 0; @@ -471,9 +479,8 @@ static void mxcmci_datawork(struct work_struct *work) mxcmci_finish_request(host, host-req); return; } - } else { + } else mxcmci_finish_request(host, host-req); - } } #ifdef HAS_DMA @@ -495,9 +502,8 @@ static void mxcmci_data_done(struct mxcmci_host *host, unsigned int stat) mxcmci_finish_request(host, host-req); return; } - } else { + } else mxcmci_finish_request(host, host-req); - } } #endif /* HAS_DMA */ @@ -512,7 +518,7 @@ static void mxcmci_cmd_done(struct mxcmci_host *host, unsigned int stat) } /* For the DMA case the DMA engine handles the data transfer -* automatically. For non DMA we have to do it ourselves. +* automatically. For non DMA we have to to it ourselves. * Don't do it in interrupt context though. */ if (!mxcmci_use_dma(host) host-data) -- 1.7.0 -- 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 3/3] ARM: MXC: mxcmmc: work around a bug in the SDHC busy line handling
MX3 SoCs have a silicon bug which corrupts CRC calculation of multi-block transfers when connected SDIO peripheral doesn't drive the BUSY line as required by the specs. One way to prevent this is to only allow 1-bit transfers. Another way is playing tricks with the DMA engine, but this isn't mainline yet. So for now, we live with the performance drawback of 1-bit transfers until a nicer solution is found. This patch introduces a new host controller callback 'init_card' which is for now only called from mmc_sdio_init_card(). Signed-off-by: Daniel Mack dan...@caiaq.de Cc: Sascha Hauer s.ha...@pengutronix.de Cc: Dan Williams dan.j.willi...@intel.com Cc: Volker Ernst volker.er...@txtr.com Cc: Jiri Kosina jkos...@suse.cz --- drivers/mmc/core/sdio.c |6 ++ drivers/mmc/host/mxcmmc.c | 14 ++ include/linux/mmc/host.h |3 +++ 3 files changed, 23 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c index 2dd4cfe..b9dee28 100644 --- a/drivers/mmc/core/sdio.c +++ b/drivers/mmc/core/sdio.c @@ -296,6 +296,12 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr, card-type = MMC_TYPE_SDIO; /* +* Call the optional HC's init_card function to handle quirks. +*/ + if (host-ops-init_card) + host-ops-init_card(host, card); + + /* * For native busses: set card RCA and quit open drain mode. */ if (!powered_resume !mmc_host_is_spi(host)) { diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c index 83a4545..f584c1b 100644 --- a/drivers/mmc/host/mxcmmc.c +++ b/drivers/mmc/host/mxcmmc.c @@ -708,11 +708,25 @@ static void mxcmci_enable_sdio_irq(struct mmc_host *mmc, int enable) spin_unlock_irqrestore(host-lock, flags); } +static void mxcmci_init_card(struct mmc_host *host, struct mmc_card *card) +{ + /* +* MX3 SoCs have a silicon bug which corrupts CRC calculation of +* multi-block transfers when connected SDIO peripheral doesn't +* drive the BUSY line as required by the specs. +* One way to prevent this is to only allow 1-bit transfers. +*/ + + if (cpu_is_mx3() card-type == MMC_TYPE_SDIO) + host-caps = ~MMC_CAP_4_BIT_DATA; +} + static const struct mmc_host_ops mxcmci_ops = { .request= mxcmci_request, .set_ios= mxcmci_set_ios, .get_ro = mxcmci_get_ro, .enable_sdio_irq= mxcmci_enable_sdio_irq, + .init_card = mxcmci_init_card, }; static int mxcmci_probe(struct platform_device *pdev) diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 43eaf5c..3196c84 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -108,6 +108,9 @@ struct mmc_host_ops { int (*get_cd)(struct mmc_host *host); void(*enable_sdio_irq)(struct mmc_host *host, int enable); + + /* optional callback for HC quirks */ + void(*init_card)(struct mmc_host *host, struct mmc_card *card); }; struct mmc_card; -- 1.7.0 -- 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: move regulator handling to core
On Thu, Dec 03, 2009 at 10:12:36PM +0200, Adrian Hunter wrote: Daniel Mack wrote: On Thu, Dec 03, 2009 at 04:27:39PM +0200, Adrian Hunter wrote: What about arch/arm/mach-omap2/mmc-twl4030.c ? Argh, missed that one. And this particular case doesn't fit to my modifications. I don't know the code well ... We would need to have a struct mmc_host * in all the functions there calling mmc_regulator_{set,get}_ocr. Any idea how to resolve that? Pass it down from the omap_hsmmc driver. It's not that easy, unfortunately, because this code does not conform to all the other mmc host drivers in tree. I don't understand why things are done the way it is currently implemented. Why isn't there a mmc_host for each slot, and why is a regulator reference acquired for each slot, and not once for the whole device? Even with the default 'vcc' supply factored out to the mmc core, the 'vmmc_aux' regulator would still need some extra attention, but I would also do that from the omap_hsmmc driver rather than in the plaform support code. Moving the regulator handling to the mmc core would require a major cleanup to all this code, but I don't have such hardware to test my modifications. Can anyone help here? Thanks, Daniel -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mmc: move regulator handling to core
At the moment, regulator operations are done from individual mmc host drivers. This is a problem because the regulators are not claimed exclusively but the mmc core enables and disables them according to the return value of regulator_is_enabled(). That can lead to a number of problems and warnings when regulators are shared among multiple consumers or if regulators are marked as 'always_on'. Fix this by moving the some logic to the core, and put the regulator reference to the mmc_host struct and let it do its own supply state tracking so that the reference counting in the regulator won't get confused. [Note that I could only compile-test the mmci.c change] Signed-off-by: Daniel Mack dan...@caiaq.de Cc: Mark Brown broo...@opensource.wolfsonmicro.com Cc: Liam Girdwood l...@slimlogic.co.uk Cc: Pierre Ossman pie...@ossman.eu Cc: Andrew Morton a...@linux-foundation.org Cc: Matt Fleming m...@console-pimps.org Cc: Adrian Hunter adrian.hun...@nokia.com Cc: David Brownell dbrown...@users.sourceforge.net Cc: Russell King rmk+ker...@arm.linux.org.uk Cc: Linus Walleij linus.wall...@stericsson.com Cc: Eric Miao eric.y.m...@gmail.com Cc: Robert Jarzmik robert.jarz...@free.fr Cc: Cliff Brake cbr...@bec-systems.com Cc: Jarkko Lavinen jarkko.lavi...@nokia.com Cc: linux-mmc@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org --- drivers/mmc/core/core.c | 36 drivers/mmc/core/host.c |3 +++ drivers/mmc/host/mmci.c | 28 drivers/mmc/host/mmci.h |1 - drivers/mmc/host/pxamci.c | 20 include/linux/mmc/host.h | 10 ++ 6 files changed, 49 insertions(+), 49 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 7dab2e5..9acb655 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -727,13 +727,13 @@ EXPORT_SYMBOL(mmc_vddrange_to_ocrmask); * regulator. This would normally be called before registering the * MMC host adapter. */ -int mmc_regulator_get_ocrmask(struct regulator *supply) +int mmc_regulator_get_ocrmask(struct mmc_host *host) { int result = 0; int count; int i; - count = regulator_count_voltages(supply); + count = regulator_count_voltages(host-vcc); if (count 0) return count; @@ -741,7 +741,7 @@ int mmc_regulator_get_ocrmask(struct regulator *supply) int vdd_uV; int vdd_mV; - vdd_uV = regulator_list_voltage(supply, i); + vdd_uV = regulator_list_voltage(host-vcc, i); if (vdd_uV = 0) continue; @@ -755,24 +755,22 @@ EXPORT_SYMBOL(mmc_regulator_get_ocrmask); /** * mmc_regulator_set_ocr - set regulator to match host-ios voltage + * @host: the mmc_host * @vdd_bit: zero for power off, else a bit number (host-ios.vdd) - * @supply: regulator to use * * Returns zero on success, else negative errno. * * MMC host drivers may use this to enable or disable a regulator using - * a particular supply voltage. This would normally be called from the + * the registered supply voltage. This would normally be called from the * set_ios() method. */ -int mmc_regulator_set_ocr(struct regulator *supply, unsigned short vdd_bit) +int mmc_regulator_set_ocr(struct mmc_host *host, unsigned short vdd_bit) { int result = 0; int min_uV, max_uV; - int enabled; - enabled = regulator_is_enabled(supply); - if (enabled 0) - return enabled; + if (!host-vcc || IS_ERR(host-vcc)) + return -EINVAL; if (vdd_bit) { int tmp; @@ -795,18 +793,24 @@ int mmc_regulator_set_ocr(struct regulator *supply, unsigned short vdd_bit) /* avoid needless changes to this voltage; the regulator * might not allow this operation */ - voltage = regulator_get_voltage(supply); + voltage = regulator_get_voltage(host-vcc); if (voltage 0) result = voltage; else if (voltage min_uV || voltage max_uV) - result = regulator_set_voltage(supply, min_uV, max_uV); + result = regulator_set_voltage(host-vcc, min_uV, max_uV); else result = 0; - if (result == 0 !enabled) - result = regulator_enable(supply); - } else if (enabled) { - result = regulator_disable(supply); + if (result == 0 !host-vcc_enabled) { + result = regulator_enable(host-vcc); + + if (result == 0) + host-vcc_enabled = 1; + } + } else
Re: [PATCH] mmc: move regulator handling to core
On Thu, Dec 03, 2009 at 01:22:41PM +, Mark Brown wrote: On Thu, Dec 03, 2009 at 02:14:23PM +0100, Daniel Mack wrote: [...] I would expect the power to be killed when the last user stops using it. Which should result in the same effect if you only have one host, one regulator, and one user. Yes, it's always fine in that case (modulo always_on and/or regulators without power control). Well, it didn't for me and always_on, though, due to the return values I described. This goes back to the thing about using regulator_get_exclusive(), the message given was that the MMC drivers really needed to be able to guarantee that the power would be removed when that was requested. Like I say, if there isn't a *strict* requirement but it's only desirable (possibly strongly desirable) then your approach is obviously preferable. The mmci people would need to answer that. To me, the code just looked like a power saving feature. If this driver needs it, the only tweak to my patch to let that particular call site use regulator_get_exclusive, and the core will still do the right thing. For this case, the behaviour should be exactly the same than it currently is, correct? Daniel -- 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: pxamci: call mmc_remove_host() before freeing resources
On Wed, Dec 02, 2009 at 08:50:44AM +0800, Eric Miao wrote: On Wed, Dec 2, 2009 at 1:17 AM, Daniel Mack dan...@caiaq.de wrote: mmc_remove_host() will cause the mmc core to switch off the bus power by eventually calling pxamci_set_ios(). This function uses the regulator or the GPIO which have been freed already. This causes the following Oops on module unload. [...] Signed-off-by: Daniel Mack dan...@caiaq.de Reported-by: Sven Neumann s.neum...@raumfeld.com Cc: Pierre Ossman pie...@ossman.eu Cc: Eric Miao eric.y.m...@gmail.com Cc: linux-mmc@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org Cc: sta...@kernel.org --- drivers/mmc/host/pxamci.c | 4 ++-- Looks like a correct fix to me. Acked-by: Eric Miao eric.y.m...@gmail.com Would be good to still get this into .32 - who will take care of picking this? -- 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: pxamci: call pdata-setpower even when regulator is used
ping. On Thu, Nov 26, 2009 at 12:10:13PM +0100, Daniel Mack wrote: On platforms where the vmmc regulator is used, it might still be necessary to call the platform data's setpower() callback in order to drive GPIOs to power up hardware. I can't see any pxamci user with power regulators enabled, hence this shouldn't break anything. Signed-off-by: Daniel Mack dan...@caiaq.de Cc: Pierre Ossman pie...@ossman.eu Cc: Eric Miao eric.y.m...@gmail.com Cc: linux-mmc@vger.kernel.org --- drivers/mmc/host/pxamci.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c index b00d673..c35df2b 100644 --- a/drivers/mmc/host/pxamci.c +++ b/drivers/mmc/host/pxamci.c @@ -84,7 +84,7 @@ static inline void pxamci_init_ocr(struct pxamci_host *host) host-mmc-ocr_avail = mmc_regulator_get_ocrmask(host-vcc); if (host-pdata host-pdata-ocr_mask) dev_warn(mmc_dev(host-mmc), - ocr_mask/setpower will not be used\n); + given ocr_mask will not be used\n); } #endif if (host-vcc == NULL) { @@ -109,7 +109,7 @@ static inline void pxamci_set_power(struct pxamci_host *host, unsigned int vdd) gpio_set_value(host-pdata-gpio_power, !!on ^ host-pdata-gpio_power_invert); } - if (!host-vcc host-pdata host-pdata-setpower) + if (host-pdata host-pdata-setpower) host-pdata-setpower(mmc_dev(host-mmc), vdd); } -- 1.6.5.2 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: MXC MMC driver and SDIO peripherals
On Wed, Oct 21, 2009 at 01:15:19PM -0700, Dan Williams wrote: On Wed, 2009-10-21 at 21:20 +0200, Daniel Mack wrote: Hi, we're having trouble getting SDIO connected harware to fly on MX31 based designs. In particular, a SD8686 chip supported by the libertas_sdio driver will hang forever when built without CONFIG_MMC_DEBUG=y. With that option selected, however, the behaviour is a little different, and I can at least see the following messages on a recent 2.6.32-rc5 based MX31 tree. Is there any common pitfall for such setups? I did more or less the same thing on PXAs (same WLAN chip, same kind of interface, same firmware), and haven't seen any such effects, so I suspect the MXC specific parts to be the reason for that. Any ideas? Any idea what quirks your SDHC is using if any? Does it require PIO or can it do DMA? Does it have any transfer restrictions on block size or bit-width? What is the debug output of the MMC stack when loading the module for your SDHC? I did some more research on this and it turns out that the problem is related to multi block transfers. At least, this is when it first occurs. The libertas SDIO driver downloads two firmwares to the device, one 'helper' and one 'real' firmware The first one only uses chunks of 64 bytes each and that seems to work fine. The real firmware, however, loads in 512 bytes chunks which the SDIO core breaks up into 16 blocks of 32 bytes. And this is where the MXC host controller bails out with a CRC error. Unfortunately, it does not give any more detailed information about what exactly went wrong. The effect might be related to an errata entry[1], which is what I'm currently investigating. To do so, I would like to limit the the communication to singe-block transfers, just to exclude all other possible (electrical, clock speed, ...) issues. I did that by setting mmc-max_blk_count to 1 in the the host controller, but then again, the libertas driver and/or the firmware doesn't like that and dies in if_sdio_pro_real() with firmware wants 17 bytes firmware helper signalled error Any idea how to get that working with only single block small transfers? Btw - there's a number of things missing for SDIO in the MXC MMC driver which I implemented/fixed. I'll send patches as soon as I have more confidence about the whole setup. Thanks, Daniel [1] Errata IDs TLSbo91748 and TLSbo78667 from http://www.freescale.com/files/32bit/doc/errata/MCIMX31CE.pdf?fpsp=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
MXC MMC driver and SDIO peripherals
Hi, we're having trouble getting SDIO connected harware to fly on MX31 based designs. In particular, a SD8686 chip supported by the libertas_sdio driver will hang forever when built without CONFIG_MMC_DEBUG=y. With that option selected, however, the behaviour is a little different, and I can at least see the following messages on a recent 2.6.32-rc5 based MX31 tree. Is there any common pitfall for such setups? I did more or less the same thing on PXAs (same WLAN chip, same kind of interface, same firmware), and haven't seen any such effects, so I suspect the MXC specific parts to be the reason for that. Any ideas? Thanks, Daniel [1.45] mmc0: new SDIO card at address 0001 ... # modprobe libertas_sdio [ 15.18] libertas_sdio: Libertas SDIO driver [ 15.18] libertas_sdio: Copyright Pierre Ossman [ 15.19] libertas_sdio mmc0:0001:1: firmware: requesting sd8686_helper.bin [ 15.32] libertas_sdio mmc0:0001:1: firmware: requesting sd8686.bin [ 15.84] libertas: 00:19:88:03:13:5a, fw 8.73.7p3, cap 0x0393 [ 16.87] libertas: problem fetching packet from firmware [ 18.84] libertas: command 0x001e timed out [ 18.84] libertas: requeueing command 0x001e due to timeout (#1) [ 18.87] libertas: Received result 0 to command 1e after 1 retries [ 18.91] libertas: wlan0: Marvell WLAN 802.11 adapter # ifconfig wlan0 up [ 216.76] ADDRCONF(NETDEV_UP): wlan0: link is not ready [ 217.80] libertas: problem fetching packet from firmware [ 219.77] libertas: command 0x0010 timed out [ 219.77] libertas: requeueing command 0x0010 due to timeout (#1) [ 219.80] libertas: Received result 0 to command 10 after 1 retries # iwlist wlan0 scan [ 244.26] libertas: problem fetching packet from firmware [ 247.80] libertas: command 0x0006 timed out [ 247.80] libertas: requeueing command 0x0006 due to timeout (#1) [ 248.26] libertas: Received result 0 to command 6 after 1 retries [ 249.92] libertas: problem fetching packet from firmware [ 253.56] libertas: command 0x0006 timed out [ 253.56] libertas: requeueing command 0x0006 due to timeout (#1) [ 254.92] libertas: problem fetching packet from firmware wlan0 Failed to read scan data : Resource temporarily unavailable -- 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: MXC MMC driver and SDIO peripherals
Hi Dan, On Wed, Oct 21, 2009 at 01:15:19PM -0700, Dan Williams wrote: On Wed, 2009-10-21 at 21:20 +0200, Daniel Mack wrote: Hi, we're having trouble getting SDIO connected harware to fly on MX31 based designs. In particular, a SD8686 chip supported by the libertas_sdio driver will hang forever when built without CONFIG_MMC_DEBUG=y. With that option selected, however, the behaviour is a little different, and I can at least see the following messages on a recent 2.6.32-rc5 based MX31 tree. Is there any common pitfall for such setups? I did more or less the same thing on PXAs (same WLAN chip, same kind of interface, same firmware), and haven't seen any such effects, so I suspect the MXC specific parts to be the reason for that. Any ideas? Any idea what quirks your SDHC is using if any? Does it require PIO or can it do DMA? In mainline kernels, DMA is limited to the MX2 SoC family. The MX3 that I'm using is excluded from that feature, but I'mm not aware of the reason for that. Does it have any transfer restrictions on block size or bit-width? I didn't really debug it yet, but from what I see in the sources, the maximum block size is 2048. The hardware is connected via the 4-wire interface (+clk, +cmd). The system I tried that on has a MMC slot where I can plug in either the WLAN eval kit or a memory card, and the latter works just fine, which tells me that at least the pin config and the hardware can't be entirely wrong. What is the debug output of the MMC stack when loading the module for your SDHC? Exactly one line ;) [1.34] i.MX SDHC driver Daniel -- 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