[RESEND PATCH v2 0/3] omap_hsmmc: SDIO IRQ on AM335x family
No changes to the patches itself.
Only the dependency on some omap-gpio enable_irq/disable_irq patch
has been removed.
While developing, I was struck by a bug with disable_irq. After reviewing
the disable_irq code path, I thought the interrrupt got never disabled
for omap. After fixing the bug I was hunting, which was completely
unrelated to disable_irq, I never verified if the dependency was really
needed.
While trying to upstream my disable_irq fixes for gpio-omap, I realized,
that disable_irq was always working for gpio-omap through the generic
lazy disable mechanism.
/**
* irq_disable - disable interrupt generation
* @desc: irq descriptor which should be disabled
*
* If the chip does not implement the irq_disable callback, we
* use a lazy disable approach. That means we mark the interrupt
* disabled, but leave the hardware unmasked. If an interrupt
* happens, then the interrupt flow handler masks the line at the
* hardware level and marks it pending.
*/
void irq_disable(struct irq_desc *desc)
{
irq_state_set_disabled(desc);
if (desc-irq_data.chip-irq_disable) {
desc-irq_data.chip-irq_disable(desc-irq_data);
irq_state_set_masked(desc);
}
}
The 4+ weeks testing mentionned in the 1st patch, was done with a
dedicated irq_disable hook in gpio-omap. I'm positive that it is not
needed at all, still the test was repeated for 1 day without that hook.
Andreas Fenkart (3):
mmc: omap_hsmmc: Enable SDIO IRQ using a GPIO in idle mode.
mmc: omap_hsmmc: debugfs entries for GPIO mode.
mmc: omap_hsmmc: add PSTATE to debugfs regs_show.
.../devicetree/bindings/mmc/ti-omap-hsmmc.txt | 42 +++
drivers/mmc/host/omap_hsmmc.c | 269 ++--
include/linux/platform_data/mmc-omap.h |4 +
3 files changed, 294 insertions(+), 21 deletions(-)
--
1.7.10.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 v2 0/3] omap_hsmmc: SDIO IRQ on AM335x family
This is a resend of an earlier patch http://comments.gmane.org/gmane.linux.ports.arm.omap/90381 It's rebased against Linux 3.9-rc6 / 31880c37c11e28cb8 Main changes are two bug fixes. Also did lots of testing. Fixes are a race condition during suspend and the gpio irq being disabled unintendedly. The gpio irq is managed via the 'disable_depth' ref counter of the irq subsystem, driver simply call enable_irq/disable_irq when needed. sdio irqsdio irq unmasked masked - runtime suspend |0 | 1 runtime default |1 | 2 irq disable depth only when sdio irq is enabled AND module is idle, the ref drops to zero and the gpio irq is effectively armed. problem was that omap_hsmmc_enable_sdio_irq(.. disable) was called when the irq was already disabled. This corrupted the ref count. Another issue was to ensure that the ref count starts with the right initial value (2). I did quite some duration testing. Test setup is two sdio wifi cards, running iperf as server/client. Test is running for 8+ days now, still running. # cat /sys/kernel/debug/mmc0/regs [snip] pinmux config sdio sdio irqenabled pm suspends 19993594 So there had been ~20 Mio sdio to gpio pinmux transitions. Without enabling the pinmux toggling in the dts file, there is no functional change. The driver will use polling as it does now. If you want to enable it, you also need to implement enable_irq/disable_irq for omap chips. https://patchwork.kernel.org/patch/1886421/ http://permalink.gmane.org/gmane.linux.ports.arm.omap/97093 /Andi Andreas Fenkart (3): mmc: omap_hsmmc: Enable SDIO IRQ using a GPIO in idle mode. mmc: omap_hsmmc: debugfs entries for GPIO mode. mmc: omap_hsmmc: add PSTATE to debugfs regs_show. .../devicetree/bindings/mmc/ti-omap-hsmmc.txt | 42 +++ drivers/mmc/host/omap_hsmmc.c | 269 ++-- include/linux/platform_data/mmc-omap.h |4 + 3 files changed, 294 insertions(+), 21 deletions(-) -- 1.7.10.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: omap_hsmmc: SDIO IRQ on AM335x family
* Andreas Fenkart andreas.fenk...@streamunlimited.com [121220 14:07]: Hi, On Fri, Nov 30, 2012 at 07:57:35PM +0100, Daniel Mack wrote: 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. I don't understand your concern here, could you explain 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? The IRQ is level triggered, so can't be lost. I implemented it as suggested and surprisingly performance is pretty good. Actually not worse than keeping the fclk enabled all times. module: 88W8787 / mwifiex tx bitrate: 150.0 MBit/s MCS 7 40Mhz short GI | tcp tx | signal | cpu idle --- keep fclk enabled | 50.3 Mbits/sec | -23 dBm | 15 % suspend/resume | 49.7 Mbits/sec | -22 dBM | 13 % patch follows Hey that's cool :) Will take a look at the patch. Tony -- 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, On Fri, Nov 30, 2012 at 07:57:35PM +0100, Daniel Mack wrote: 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. I don't understand your concern here, could you explain 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? The IRQ is level triggered, so can't be lost. I implemented it as suggested and surprisingly performance is pretty good. Actually not worse than keeping the fclk enabled all times. module: 88W8787 / mwifiex tx bitrate: 150.0 MBit/s MCS 7 40Mhz short GI | tcp tx | signal | cpu idle --- keep fclk enabled | 50.3 Mbits/sec | -23 dBm | 15 % suspend/resume | 49.7 Mbits/sec | -22 dBM | 13 % patch follows /Andi -- 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
omap_hsmmc: SDIO IRQ on AM335x family
Hi, I submitted following patches a while back, http://www.spinics.net/lists/linux-mmc/msg17624.html. Since there was no feedback I'm taking a step back, documenting: 1. why is it needed, missing swakeup line 2. transition, enable workaround by default 3. device tree configuration [why is it needed, problem outline] The omap_hsmmc module is suspended whenever it is idle, its functional clock being turned off. In this mode it is not able to forwared IRQs to the system. For that to happen, it needs to tell the PRCM to restore it's fclk. -- | PRCM | -- | ^ fclk | | swakeup v | --- -- -- IRQ -- | hsmmc | -- CIRQ -- | card | --- -- This is done through the swakeup line, which can be configured to trigger for various events, among others; CIRQ. The problem is that on the AM335x family the swakeup line is missing, it has not been routed from the module to the PRCM. https://www.google.com/search?q=spruh73c see 18.2 compare with 20.2.2, or just search the whole doc for swakeup One solution is to keep the fclk enabled while waiting for CIRQ. This is the approach taken by existing patches: http://www.spinics.net/lists/linux-omap/msg63363.html http://marc.info/?l=linux-mmcm=126580279403824w=2 http://thread.gmane.org/gmane.linux.kernel.mmc/1107/focus=1109 authors, contributors David Vrabel (based my work on it) John Rigby 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. [transition, enable workaround by default?] Up to now the driver uses polling, which works for all SOCs. Once we enable the IRQ, all chips missing an swakeup will fail. But I don't know all of them, so how to transition without breaking existing platforms? Enable IRQ only for AM335x, use polling for others? Keep clock running for all, only disable workaround for known good platforms? [device tree configuration] My previous patch, see link at top, uses a new property 'ti,swakeup-not-implemented'. Then in the DT files I enabled the workaround for my platform. Now I learned that I should rather use a new 'compatible' section. Actually Daniel talked me out of it: 'Since this feature should always be enabled on one given processor, the IP block is actually compatible to something else than a processor that doesn't support that feature. So the compatible line alone should be enough of an information for the one who provides the DT. On the other hand, if enabling a certain option depends on the system design of an application, it deserves a special binding in DT.' I'd prefer the latter now. Thanks for your attention, /Andi -- 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
* 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. Regards, Tony -- 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
