Re: mmc: sdio: runtime PM and 8686 problems

["Joe Woodward" <jw@terrafix.co.uk>]

-----Original Message-----
From: Steve Sakoman <sakoman@gmail.com>
To: Ohad Ben-Cohen <ohad@wizery.com>
Cc: Chris Ball <cjb@laptop.org>, Joe Woodward <jw@terrafix.co.uk>, linux-mmc@vger.kernel.org, Neil Brown <neilb@suse.de>, Bing Zhao 
<bzhao@marvell.com>, Daniel Drake <dsd@laptop.org>
Date: Thu, 26 Apr 2012 16:51:10 -0700
Subject: Re: mmc: sdio: runtime PM and 8686 problems

> On Mon, Dec 19, 2011 at 1:10 AM, Ohad Ben-Cohen <ohad@wizery.com>
> wrote:
> 
> > We're slowly progressing towards understanding the issues we have
> with
> > the 8686 (it seems there are a few different hw revisions of it, some
> > of which do require toggling the reset pin before sending another
> init
> > sequence), but it might take some more time until a solution fully
> > materializes.
> >
> > Since 3.2 is just around the corner, I suggest we should now proceed
> > with the revert (see patch below), and later revisit this once we
> have
> > a complete solution in hand.
> 
> Has any progress been made on this issue?
> 
> I too have been trying to power down/up the wifi module on the Overo.
> I'm seeing the same issue described in this thread.
> 
> Some additional data:
> 
> I rebuilt my kernel (3.2) with a CD GPIO enabled for the mmc port the
> module
> is connected to and in turn connected that GPIO to another one that I
> can toggle from user space.
> 
> Toggling the CD GPIO after powering back up the module does indeed
> work properly -- the module is detected, the driver loaded, and proper
> function
> restored.
> 
> So basically the procedure I've used is:
> 
> - decide wifi can be powered down
> - unload the libertas modules
> - put the wifi hw in reset with gpio16
> - wait till wifi is needed again
> - take the chip out of reset
> - toggle the CD gpio
> - libertas modules are autoloaded, as is the firmware
> - wifi is up and functioning
> 
> So the next step is to get rid of the silly two GPIO external hardware
> hack.  Is it possible to trigger a card insertion/removal event via
> some standard API?
> 
> Or does the above information provide a clue as to why normal code
> paths don't work?
> 
> Any other ideas?
> 
> Steve
> 

Since the patch has been reverted (i.e. no runtime PM) I haven't seen a problem.

I've not put any more work in to getting it to work with runtime PM i'm afriad.

On a side note, I'm doing something very similar to you with my Overo, except with one difference. Where you have routed a GPIO externally which you can toggle 
from userspace, I've hacked the MMC driver (omap_hsmmc.c) to add a sysfs entry to force a re-scan. This also allows me to safely remove/insert SD cards (as the 
Overo SDCard slot doesn't have a card-detect pin) when the device is running (I run from a ramfs).

I've thought for a while that a userspace hook to kick the MMC driver where this is no physical GPIO would be useful in mainline, but haven't done any more about 
it.

Also, I've appled the first pass of patches to enable Libertas async firmware loading (as required by newer UDEVs).

I have two scripts pasted below to turn on/off the WiFi (the delays just work for me, I know they are not optimal).

Hope this helps.

Cheers,
Joe


> cat #wifi-down.sh
#!/bin/sh

echo "Turning off WiFi..."

echo "...stopping wlan0 interface"
ifdown wlan0

echo "...unloading WiFi kernel module"
modprobe -r libertas_sdio

echo "...disabling power to WiFi chip"
if [ -e /sys/class/gpio/gpio16/value ] ; then
  echo "...already exported GPIO reset control"
else
  echo "...exporting GPIO reset control"
  echo 16 > /sys/class/gpio/export
  echo out > /sys/class/gpio/gpio16/direction
fi
echo 0 > /sys/class/gpio/gpio16/value

echo "...removing wpa configuration file"
rm /tmp/wpa_supplicant.conf

echo "..rescanning the MMC/SDIO as we have powered down the WiFi chip"
echo 1 > /sys/bus/platform/drivers/omap_hsmmc/omap_hsmmc.1/mmc_host/mmc1/rescan
sleep 2

echo "...done"



#> cat wifi-up.sh
#!/bin/sh

# Get the network parameters.
SSID=$1
PSK=$2

echo "Turning on WiFi..."

echo "...enabling power to WiFi chip"
if [ -e /sys/class/gpio/gpio16/value ] ; then
  echo "...already exported GPIO reset control"
else
  echo "...exporting GPIO reset control"
  echo 16 > /sys/class/gpio/export
  echo out > /sys/class/gpio/gpio16/direction
fi
echo 0 > /sys/class/gpio/gpio16/value
usleep 10
echo 1 > /sys/class/gpio/gpio16/value
usleep 10

echo "..rescanning the MMC/SDIO as we have powered up the WiFi chip"
echo 1 > /sys/bus/platform/drivers/omap_hsmmc/omap_hsmmc.1/mmc_host/mmc1/rescan

### Wait for the device to be found.
sleep 2

### Set up the AP and password.
echo "...adding wpa configuration file"
rm /tmp/wpa_supplicant.conf
echo "ctrl_interface=/var/run/wpa_supplicant
ctrl_interface_group=0
eapol_version=1
ap_scan=1
fast_reauth=1

network={
      ssid=\"${SSID}\"
      proto=WPA2                 # try WPA RSN if you WPA2 fails
      key_mgmt=WPA-PSK
      pairwise=CCMP TKIP
      group=CCMP TKIP
      scan_ssid=1
      psk=\"${PSK}\"
      priority=10
}" > /tmp/wpa_supplicant.conf

echo "...loading WiFi kernel module"
modprobe libertas_sdio

### We must wait for the asynchronous firmware loading to complete.
sleep 2

echo "...starting wlan0 interface"
ifup wlan0

echo "...done"


=========
On 3.2 the rescan looks a bit like:

static ssize_t
omap_hsmmc_store_rescan(struct device *dev,
		struct device_attribute *attr, const char *buf, size_t size)
{
	struct mmc_host *mmc = container_of(dev, struct mmc_host, class_dev);
	struct omap_hsmmc_host *host = mmc_priv(mmc);

printk (KERN_ERR "MMC: rescanning...\n");

	if (!host->suspended)
		schedule_work(&host->mmc_carddetect_work);

	return size;
}

static DEVICE_ATTR(rescan, S_IWUGO, NULL, omap_hsmmc_store_rescan);


And I add in to omap_hsmmc_probe:

	ret = device_create_file(&mmc->class_dev, &dev_attr_rescan);
	if (ret < 0)
		goto err_slot_name;


> 
> >> When SDIO runtime PM was originally introduced, we immediately faced
> >> two regressions with two different chipsets, and in response decided
> >> not to enable it by default.
> >>
> >> With your work on the 8686 we hoped we found all the gotchas, so
> >> 08da834 did make sense (at least experimentally).
> >>
> >> Unfortunately we now see that some setups out there still refuse to
> >> work when SDIO runtime PM is enabled by default, and obviously we
> >> can't live with these kind of regressions.
> >>
> >> commit fd9fec7a00f58a16803e37a99a014ef82543ef6f
> >> Author: Ohad Ben-Cohen <ohad@wizery.com>
> >> Date:   Wed Nov 30 16:22:13 2011 +0200
> >>
> >>    Revert "mmc: enable runtime PM by default"
> >>
> >>    When SDIO runtime PM was originally introduced, we immediately
> faced
> >>    two regressions with two different chipsets, and in response
> decided
> >>    not to enable it by default.
> >>
> >>    With the recent work on the 8686 we hoped we found all the
> gotchas,
> >>    so 08da834 did make sense (at least experimentally).
> >>
> >>    Unfortunately we now see that some setups out there still
> refuse to work
> >>    when SDIO runtime PM is enabled by default (see
> >>    http://www.spinics.net/lists/linux-mmc/msg11161.html), and
> obviously we
> >>    can't live with these kind of regressions.
> >>
> >>    This reverts commit 08da834a24312157f512224691ad1fddd11c1073.
> >>
> >>    Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> >>    Cc: Daniel Drake <dsd@laptop.org>
> >>
> >> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> >> index e8a5eb3..d31c78b 100644
> >> --- a/drivers/mmc/core/host.c
> >> +++ b/drivers/mmc/core/host.c
> >> @@ -302,17 +302,6 @@ struct mmc_host *mmc_alloc_host(int extra,
> struct
> >> device *dev)
> >>        host->max_blk_size = 512;
> >>        host->max_blk_count = PAGE_CACHE_SIZE / 512;
> >>
> >> -       /*
> >> -        * Enable runtime power management by default. This flag
> was added due
> >> -        * to runtime power management causing disruption for
> some users, but
> >> -        * the power on/off code has been improved since then.
> >> -        *
> >> -        * We'll enable this flag by default as an experiment,
> and if no
> >> -        * problems are reported, we will follow up later and
> remove the flag
> >> -        * altogether.
> >> -        */
> >> -       host->caps = MMC_CAP_POWER_OFF_CARD;
> >> -
> >>        return host;
> >>
> >>  free:
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-mmc"
> in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html