[V2] mmc: don't request CD IRQ until mmc_start_host()

From: Stephen Warren <swarren@nvidia.com>

From: Stephen Warren <swarren@nvidia.com>

As soon as the CD IRQ is requested, it can trigger, since it's an
externally controlled event. If it does, delayed_work host->detect will
be scheduled.

Many host controller probe()s are roughly structured as:

*_probe() {
    host = sdhci_pltfm_init();
    mmc_of_parse(host->mmc);
    rc = sdhci_add_host(host);
    if (rc) {
        sdhci_pltfm_free();
        return rc;
    }

In 3.17, CD IRQs can are enabled quite early via *_probe() ->
mmc_of_parse() -> mmc_gpio_request_cd() -> mmc_gpiod_request_cd_irq().

Note that in linux-next, mmc_of_parse() calls mmc_gpio*d*_request_cd()
rather than mmc_gpio_request_cd(), and mmc_gpio*d*_request_cd() doesn't
call mmc_gpiod_request_cd_irq(). However, this issue still exists if
mmc_gpio_request_cd() is called directly before mmc_start_host().

sdhci_add_host() may fail part way through (e.g. due to deferred
probe for a vmmc regulator), and sdhci_pltfm_free() does nothing to
unrequest the CD IRQ nor cancel the delayed_work. sdhci_pltfm_free() is
coded to assume that if sdhci_add_host() failed, then the delayed_work
cannot (or should not) have been triggered.

This can lead to the following with CONFIG_DEBUG_OBJECTS_* enabled, when
kfree(host) is eventually called inside sdhci_pltfm_free():

WARNING: CPU: 2 PID: 6 at lib/debugobjects.c:263 debug_print_object+0x8c/0xb4()
ODEBUG: free active (active state 0) object type: timer_list hint: delayed_work_timer_fn+0x0/0x18

The object being complained about is host->detect.

There's no need to request the CD IRQ so early; mmc_start_host() already
requests it. For most SDHCI hosts at least, the typical call path that
does this is: *_probe() -> sdhci_add_host() -> mmc_add_host() ->
mmc_start_host().

Ideally, we would solve this by removing the call to
mmc_gpiod_request_cd_irq() from mmc_gpio_request_cd(). This would match
mmc_gpio*d*_request_cd(), which already doesn't call
mmc_gpiod_request_cd_irq(). However, we need to keep the call and make it
conditional; some host controller drivers call mmc_gpio_request_cd()
after mmc_start_host() has already been called, and assume that this will
also call mmc_gpiod_request_cd_irq(). Unfortunately, the only way I could
find to differentiate the two cases was to add an extra parameter to
mmc_gpio_request_cd() to provide this information.

This solves the problem (eliminates the kernel error message above),
since it guarantees that the IRQ can't trigger before mmc_start_host()
is called.

The critical point here is that once sdhci_add_host() calls
mmc_add_host() -> mmc_start_host(), sdhci_add_host() is coded not to
fail. In other words, if there's a chance that mmc_start_host() may have
been called, and CD IRQs triggered, and the delayed_work scheduled,
sdhci_add_host() won't fail, and so cleanup is no longer via
sdhci_pltfm_free() (which doesn't free the IRQ or cancel the work queue)
but instead must be via sdhci_remove_host(), which calls mmc_remove_host()
-> mmc_stop_host(), which does free the IRQ and cancel the work queue.

CC: Russell King <linux@arm.linux.org.uk>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexandre Courbot <acourbot@nvidia.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
Note: This is a patch for v3.17 (and perhaps earlier); linux-next doesn't
have this problem due to other code re-structing. However, I think those
patches are too large to backport.

v2: Rather than completely removing mmc_gpio_request_cd()'s call to
mmc_gpiod_request_cd_irq(), make the call conditional.
---
 drivers/mmc/core/host.c            |  2 +-
 drivers/mmc/core/slot-gpio.c       | 13 +++++++++++--
 drivers/mmc/host/jz4740_mmc.c      |  3 ++-
 drivers/mmc/host/mmc_spi.c         |  2 +-
 drivers/mmc/host/mmci.c            |  2 +-
 drivers/mmc/host/mvsdio.c          |  2 +-
 drivers/mmc/host/sdhci-esdhc-imx.c |  3 ++-
 drivers/mmc/host/sdhci-pxav3.c     |  2 +-
 drivers/mmc/host/sdhci-sirf.c      |  2 +-
 drivers/mmc/host/sdhci-spear.c     |  3 ++-
 drivers/mmc/host/sh_mmcif.c        |  2 +-
 drivers/mmc/host/tmio_mmc_pio.c    |  2 +-
 include/linux/mmc/slot-gpio.h      |  2 +-
 13 files changed, 26 insertions(+), 14 deletions(-)

[V2] mmc: don't request CD IRQ until mmc_start_host()

Commit Message

Comments

Patch