Message-ID: <20111210181546.6da7099a@notabene.brown>
Date:	Sat, 10 Dec 2011 18:15:46 +1100
From:	NeilBrown <neilb@...e.de>
To:	Ohad Ben-Cohen <ohad@...ery.com>
Cc:	linux-mmc@...r.kernel.org, lkml <linux-kernel@...r.kernel.org>,
	Daniel Drake <dsd@...top.org>,
	Joe Woodward <jw@...rafix.co.uk>, Chris Ball <cjb@...top.org>,
	Bing Zhao <bzhao@...vell.com>
Subject: Re: [PATCH] mmc/sdio: don't allow interface to runtime-suspend
 until probe is finished.

On Mon, 5 Dec 2011 21:37:07 +0200 Ohad Ben-Cohen <ohad@...ery.com> wrote:

> On Mon, Dec 5, 2011 at 9:06 PM, NeilBrown <neilb@...e.de> wrote:
> > Challenge accepted.
> 
> cool :)
> 
> > 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.
> 
> I somewhat tend to pick a white listing approach, so we don't keep on
> inflicting further pain on random sdio users (at least until we learn
> more about the issues at hand), but I wouldn't object black listing.
> 
> > 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.
> 
> Well, not according to Bing Zhao from Marvell:
> 
> http://www.mail-archive.com/linux-mmc@vger.kernel.org/msg08257.html
> 
> Though I agree that if we do find this reset line helping (and get it
> to work cleanly with the help of some regulator code), the solution
> might be much more robust.
> 
> > "It *should be* perfectly fine ..." :-)
> 
> heh, yeah :)
> 
> > 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.
> 
> Btw did you try toggling the reset line manually and see if it really helps ?
> 
> > I'll spend some more time on this and get back to you - probably next week.
> 
> Very cool, thanks a lot.
> 
> Ohad.

Well... I've got something to report.
 

Firstly, this has nothing to do with power-supply to the chip.  The regulator
stays on the whole time and I still need to reset as some point.  However
there is no difference between:
  - power on with reset high
  - power on with reset low, then raise reset.
So what I thought about reset before is wrong as you say.

Secondly, there are two ways that I can get the wifi to work.

1/ If I keep the device from suspending until the functions have been probed,
   it works.

 To expand on this a bit: when I allow it to sleep early I see messages like:

[  315.347930] bus: 'sdio': driver_probe_device: matched device mmc1:0001:1 with driver libertas_sdio
[  315.368713] bus: 'sdio': really_probe: probing driver libertas_sdio with device mmc1:0001:1
[  315.380859] mmc1: mmc_power_restore_host: powering up
[  315.478668] libertas_sdio: probe of mmc1:0001:1 failed with error -16

This error is ultimately caused by mmc_send_io_op_cond called from
mmc_sdio_power_restore returning -110.

When I force it to hold off suspend for a little while I see (starting at the
same place):

[  656.189697] bus: 'sdio': driver_probe_device: matched device mmc1:0001:1 with driver libertas_sdio
[  656.212768] bus: 'sdio': really_probe: probing driver libertas_sdio with device mmc1:0001:1
[  656.247741] libertas_sdio mmc1:0001:1: firmware: requesting libertas/sd8686_v9_helper.bin
[  656.257537] device: 'mmc1:0001:1': device_add
[  656.263580] PM: Adding info for No Bus:mmc1:0001:1
[  656.298187] device: 'mmc1:0001:1': device_unregister
[  656.303375] PM: Removing info for No Bus:mmc1:0001:1
[  656.322967] libertas_sdio mmc1:0001:1: firmware: requesting libertas/sd8686_v8_helper.bin
[  656.344207] device: 'mmc1:0001:1': device_add
[  656.357147] PM: Adding info for No Bus:mmc1:0001:1
[  656.395782] device: 'mmc1:0001:1': device_unregister
[  656.407409] PM: Removing info for No Bus:mmc1:0001:1
[  656.418579] libertas_sdio mmc1:0001:1: firmware: requesting sd8686_helper.bin
[  656.435699] device: 'mmc1:0001:1': device_add
[  656.446655] PM: Adding info for No Bus:mmc1:0001:1
[  656.504974] device: 'mmc1:0001:1': device_unregister
[  656.511749] PM: Removing info for No Bus:mmc1:0001:1
[  656.521148] libertas_sdio mmc1:0001:1: firmware: requesting sd8686.bin
[  656.529205] device: 'mmc1:0001:1': device_add
[  656.535095] PM: Adding info for No Bus:mmc1:0001:1
[  656.584625] device: 'mmc1:0001:1': device_unregister
[  656.591369] PM: Removing info for No Bus:mmc1:0001:1
[  657.384063] libertas_sdio mmc1:0001:1: (unregistered net_device): 00:19:88:3d:ff:f0, fw 9.70.3p24, cap 0x00000303
[  657.454467] libertas_sdio mmc1:0001:1: (unregistered net_device): PREP_CMD: command 0x00a3 failed: 2
[  657.464080] device: 'phy0': device_add


 But shortly there after the extra tracing I put in shows that mmc_power_off
 is called, then mmc_sdio_power_restore calls mmc_send_io_op_cond which again
 returns -110, but now it isn't a problem and the wifi chip keeps working.
 So maybe the fact that we error-out in the first case is a problem??


 I found that if I pull the reset line down and then let it back up then it
 all works.  mmc_send_io_op_cond never returns -110.
 Specifically I first put the reset (
	gpio_set_value_cansleep(212, 0);
	mmc_delay(10);
	gpio_set_value_cansleep(212, 1);
	mmc_delay(10);

  ) in mmc_power_up just before the final "mmc_host_clk_release(host);"
  That worked.

  I then tried to be clever and defined a 'remux' routine (called 
  from hsmmc23_before_set_reg
  from omap_hsmmc_235_set_power
  from omap_hsmmc_set_io
  from mmc_power_up), and that works too.

  So that allows me to reset the wifi chip at the right time with only
  changes to my 'board' file.

  However I don't think a reset was the intention of 'remux' so this solution
  feels a little dirty.

My thought is that something in the suspend/resume sequence is confusing the
wifi chip and it needs a reset to get out of it.
However if I just make mmc_power_off a no-op (insert an early 'return') it
still fails.  I tried poking around and commenting out other function calls
but only ever manage to break it more, never to fix it :-(

So I have run out of ideas.  I can make it work by reseting the chip during
mmc_power_up but I have no idea what is causing the chip to need a reset.
All I know is that it isn't the power-supply being removed.

If you or anyone would like to suggest anything else I could test, or
any interesting information that I could extract that might be helpful, then
I'm happy to give that a go (with the understanding that the turn-around time
could easily be a week or more).

However for now I think I'll go with my 'remux' hack.

Thanks,
NeilBrown



Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)

Powered by blists - more mailing lists