On Tue, Apr 15, 2014 at 11:28 AM, Stephen Warren <swar...@wwwdotorg.org> wrote: > On 04/14/2014 07:42 PM, Andrew Bresticker wrote: >> If regulator_get_optional() returns EPROBE_DEFER, it indicates >> that the regulator may show up later (e.g. the DT property is >> present but the corresponding regulator may not have probed). >> Instead of continuing without the regulator, return EPROBE_DEFER >> from sdhci_add_host(). Also, fix regulator leaks in the error >> paths in sdhci_add_host(). > >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > >> if (IS_ERR_OR_NULL(host->vqmmc)) { > > This is a pre-existing condition, but ick: Why doesn't this test either > IS_ERR() /or/ == NULL, but not both. On error, the regulator API should > either return and error-point or return NULL, so that client code > shouldn't need to check for both.
Unfortunately, this is necessary. regulator_get() returns NULL if !CONFIG_REGULATOR and we do not want to call regulator_is_supported_voltage() in this case since it will always return false, causing sdhci_add_host() to disable UHS modes. Alternatively, we could put the regulator_is_supported_voltage() check wihin an #ifdef CONFIG_REGULATOR, as is done when checking supported vmmc voltages below. > >> +put_vmmc: >> + if (host->vmmc) >> + regulator_put(host->vmmc); >> +put_vqmmc: >> + if (host->vqmmc) >> + regulator_put(host->vqmmc); > > If IS_ERR_OR_NULL() really is required above, it should be used here > too. More likely, I hope you need to replace if (host->vmmc) with if > (!IS_ERR(host->vmmc)). host->vmmc and host->vqmmc are set to NULL in the case of IS_ERR(), so the IS_ERR() check here isn't necessary. > > I wonder if fixing this would help solve the crashes that Alex saw? I believe the crash Alex is seeing is due to a race between tearing down the sdhci driver on probe deferral and the card-detect IRQ. Looking at it now. -- 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/