On Mon, 5 Dec 2011 12:25:42 +0200 Ohad Ben-Cohen <ohad@xxxxxxxxxx> wrote: > Hi Neil, > > On Mon, Dec 5, 2011 at 3:54 AM, NeilBrown <neilb@xxxxxxx> wrote: > > I've been chasing down a problem with the SDIO-attached wifi card in the > > GTA04 (www.gta04.org). > > 8686, right ? yep, that's the one! > > > The problem seems very similar to that described in > > > > http://www.mail-archive.com/linux-mmc@xxxxxxxxxxxxxxx/msg04289.html > > Don't go that far back, Joe just reported this exact problem in : > > http://comments.gmane.org/gmane.linux.kernel.mmc/11231 Indeed. > > > commit 08da834a24312157f512224691ad1fddd11c1073 > > Author: Daniel Drake <dsd@xxxxxxxxxx> > > Date: Wed Jul 20 17:39:22 2011 +0100 > > > > mmc: enable runtime PM by default > > > > > > and if I revert that commit so that runtime PM is not enabled the problem > > goes away. > > And this is most likely what we're going to do, unless someone with > the 8686 can further look into the problem. Challenge accepted. Even if I don't find a better solution, this seems backwards. Sure the default should be that PM is enabled, but individual board can request no PM on MMC interfaces where it is a problem. > > > (The wifi chip is a libertas_sdio supported chip connected to an mmc > > interface on an omap3, and has a separate regulator for power supply, plus a > > GPIO pin for reset, just like in the email thread. The problem is > > exemplified by: > > [ 64.643981] libertas_sdio: probe of mmc1:0001:1 failed with error -16 > > ). > > Yes, the same issue that Joe is seeing. > > > I finally managed to track down exactly what was happening - runtime PM was > > putting the interface to sleep before the SDIO functions were probed so > > initialising them failed. > > Yeah, but the question is why it fails. The chip has a requirement that the reset line is held down during power-on, and raised shortly after (I don't know exactly how short). So if you just remove power and give it back, the chip doesn't come up properly. > > It's perfectly fine to power the card down before the functions are > probed, because just before they are probed the bus is responsible to > power the card up again. "It *should be* perfectly fine ..." :-) We just have to make sure the bug powers up the card properly. Maybe I need to create a virtual regulator that powers on the real regulator, then raises the reset line. I wonder how hard that is. > > > From: NeilBrown <neilb@xxxxxxx> > > Date: Mon, 5 Dec 2011 11:34:47 +1100 > > Subject: [PATCH] mmc/sdio: don't allow interface to runtime-suspend until probe is finished. > > You can't tell when probe is finished. In fact, in can happen very far > in the future or even never at all (e.g. when the function driver > isn't loaded). Ahhh... I naively assumed that /* * ...then the SDIO functions. */ for (i = 0;i < funcs;i++) { err = sdio_add_func(host->card->sdio_func[i]); if (err) goto remove_added; } would add all the functions. but as you say: the drivers might not be loaded yet. > > > mmc_attach_sdio currently enables runtime power management on > > the mmc interface before it completes the probe of functions. > > This means that it might be asleep during the probe and the probe > > will fail. > > No - the sdio bus powers the card up before probing the drivers. See > sdio_bus_probe. > > > In particular, if 'drivers_autoprobe' is enabled on the mmc bus, then > > device_attach() will be called by bus_probe_device() from device_add() > > and it will pm_runtime_get_noresume()/pm_runtime_put_sync(). > > > > As runtime power management this the device is running > > (pm_runtime_set_active() in mmc_attach_sdio()) and rtpm is enabled > > (pm_runtime_enable()), the pm_runtime_put_sync() call puts the mmc > > interface to sleep, > > This is fine > > > so function probing fails. > > The question is why. Again - sdio_bus_probe should power up the card. > For some reason, it (or something else) fails to do so with the 8686 > on some setups. Other chips might have similar issues, too - we just > don't know (all we know is that it works great with the wl12xx, and > with the Daniel's 8686 setup). > > > So this patch moves the call to pm_runtime_enable to after all the > > calls to sdio_add_func. > > Looks like this will have the undesired side-effect of keeping the > chip powered up, even if it doesn't have any driver loaded for it. > > And this doesn't really address the problem: we still don't know why > the 8686 fails to power up at this point. > > Can you please tell us where exactly is the first failure coming from > ? is this from libertas own probe function ? is this sdio_bus_probe > getting an error from calling pm_runtime_get_sync ? > > Thanks, > Ohad I'll spend some more time on this and get back to you - probably next week. Thanks for the hints and perspective. NeilBrown
Attachment:
signature.asc
Description: PGP signature
