public inbox for linux-mmc@vger.kernel.org
 help / color / mirror / Atom feed
* Re: mmc: sdio: runtime PM and 8686 problems
@ 2011-12-01 11:29 Joe Woodward
  0 siblings, 0 replies; 26+ messages in thread
From: Joe Woodward @ 2011-12-01 11:29 UTC (permalink / raw)
  To: Ohad Ben-Cohen; +Cc: Daniel Drake, linux-mmc, Chris Ball

I've had a play with the nRESET line (as far as I can tell it is the only pin I have access to), and no joy.

Also tried playing about a bit with the CMD sequence, again no joy.

I'm not entirely sure of the CMD sequence. It seems we get:
  - CMD52 (x2)
  - CMD0
  - CMD8 (fails)
  - CMD5 (fails)

In the "SDIO Simplified Specification Version 2.00" (https://www.sdcard.org/developers/overview/sdio/sdio_spec/Simplified_SDIO_Card_Spec.pdf) page 6 it seems only a CMD52 should be required to "Re-init IO". Previous thread posts state CMD5 (x2) are required by the 8686, but why CMD0/CMD8?

Incidentally errors only occur after the CMD0, but before the first error we get (i.e. a change of cs from 1 to 0):
"mmc1: clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 20 width 0 timing 0".

Any thoughts?

I think we've probably hit the end of the line here, unless someone else pops up who knows more about the Gumstix (i.e. has access to a schematic) or the W2CBW003 and it's SD8686 internals.

Thanks for the help anyway chaps!

Cheers,
Joe


-----Original Message-----
From: Ohad Ben-Cohen <ohad@wizery.com>
To: Joe Woodward <jw@terrafix.co.uk>
Cc: Daniel Drake <dsd@laptop.org>, linux-mmc@vger.kernel.org, Chris Ball <cjb@laptop.org>
Date: Thu, 1 Dec 2011 10:07:24 +0200
Subject: Re: mmc: sdio: runtime PM and 8686 problems

> On Wed, Nov 30, 2011 at 5:00 PM, Joe Woodward <jw@terrafix.co.uk>
> wrote:
> > It seems CMD5 fails (which seems to be what the workarounds were
> aiming to fix).
> 
> Right; you get a timeout (-110), which means the 8686 didn't send any
> response.
> 
> This is unexpected because the sdio reset (the CMD52 you see before
> the init sequence) should be enough to reset the device (we confirmed
> this with the Marvell folks on this list awhile ago).
> 
> > I'm afraid I may not be of much more use as Gumstix (annoyingly)
> don't release schematics of their COMs, so actually probing any signals
> isn't really possible.
> 
> Yes, it makes it difficult to debug.
> 
> You can try to verify (in the code) that all the text book
> prerequisites are there (e.g. all SDIO lines are pulled up and muxed
> correctly. though since stuff work for you without SDIO runtime PM, I
> assume these are ok), and maybe even play a bit with stuff like
> delays, sdio resets and power cycles and see if things change. If you
> have access to the other inputs of the 8686 (it has several of them
> which are related to power and resets. Neither me nor Daniel have
> intimate knowledge about what any of those really do, but you could
> dig some info from emails send by Marvell folks on this list), then
> you can also try to play with them and see if things change. With
> luck, you could learn more about the problem and can hopefully get
> closer to finding a solution.
> 
> It could be great if a solution will be found, but if not, we'll
> probably have to proceed with the revert...
> 
> Good luck!
> Ohad.
> --
> 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



^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: mmc: sdio: runtime PM and 8686 problems
@ 2011-12-01 11:29 Joe Woodward
  0 siblings, 0 replies; 26+ messages in thread
From: Joe Woodward @ 2011-12-01 11:29 UTC (permalink / raw)
  To: Ohad Ben-Cohen; +Cc: Daniel Drake, linux-mmc, Chris Ball

I've had a play with the nRESET line (as far as I can tell it is the only pin I have access to), and no joy.

Also tried playing about a bit with the CMD sequence, again no joy.

I'm not entirely sure of the CMD sequence. It seems we get:
  - CMD52 (x2)
  - CMD0
  - CMD8 (fails)
  - CMD5 (fails)

In the "SDIO Simplified Specification Version 2.00" (https://www.sdcard.org/developers/overview/sdio/sdio_spec/Simplified_SDIO_Card_Spec.pdf) page 6 it seems only a CMD52 should be required to "Re-init IO". Previous thread posts state CMD5 (x2) are required by the 8686, but why CMD0/CMD8?

Incidentally errors only occur after the CMD0, but before the first error we get (i.e. a change of cs from 1 to 0):
"mmc1: clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 20 width 0 timing 0".

Any thoughts?

I think we've probably hit the end of the line here, unless someone else pops up who knows more about the Gumstix (i.e. has access to a schematic) or the W2CBW003 and it's SD8686 internals.

Thanks for the help anyway chaps!

Cheers,
Joe


-----Original Message-----
From: Ohad Ben-Cohen <ohad@wizery.com>
To: Joe Woodward <jw@terrafix.co.uk>
Cc: Daniel Drake <dsd@laptop.org>, linux-mmc@vger.kernel.org, Chris Ball <cjb@laptop.org>
Date: Thu, 1 Dec 2011 10:07:24 +0200
Subject: Re: mmc: sdio: runtime PM and 8686 problems

> On Wed, Nov 30, 2011 at 5:00 PM, Joe Woodward <jw@terrafix.co.uk>
> wrote:
> > It seems CMD5 fails (which seems to be what the workarounds were
> aiming to fix).
> 
> Right; you get a timeout (-110), which means the 8686 didn't send any
> response.
> 
> This is unexpected because the sdio reset (the CMD52 you see before
> the init sequence) should be enough to reset the device (we confirmed
> this with the Marvell folks on this list awhile ago).
> 
> > I'm afraid I may not be of much more use as Gumstix (annoyingly)
> don't release schematics of their COMs, so actually probing any signals
> isn't really possible.
> 
> Yes, it makes it difficult to debug.
> 
> You can try to verify (in the code) that all the text book
> prerequisites are there (e.g. all SDIO lines are pulled up and muxed
> correctly. though since stuff work for you without SDIO runtime PM, I
> assume these are ok), and maybe even play a bit with stuff like
> delays, sdio resets and power cycles and see if things change. If you
> have access to the other inputs of the 8686 (it has several of them
> which are related to power and resets. Neither me nor Daniel have
> intimate knowledge about what any of those really do, but you could
> dig some info from emails send by Marvell folks on this list), then
> you can also try to play with them and see if things change. With
> luck, you could learn more about the problem and can hopefully get
> closer to finding a solution.
> 
> It could be great if a solution will be found, but if not, we'll
> probably have to proceed with the revert...
> 
> Good luck!
> Ohad.
> --
> 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



^ permalink raw reply	[flat|nested] 26+ messages in thread
* mmc: sdio: runtime PM and 8686 problems
@ 2011-11-18  8:53 Joe Woodward
  2011-11-18  9:06 ` Ohad Ben-Cohen
  0 siblings, 1 reply; 26+ messages in thread
From: Joe Woodward @ 2011-11-18  8:53 UTC (permalink / raw)
  To: linux-mmc

(resending as my mailer seems to have decided that the first try was a reply to "RE: [PATCH 2/2] mmc: core: Support packed command for eMMC4.5 
device,"... apologies!)

I have a GUMSTIX Overo (OMAP35xx) with a Marvel 8686 WiFi attached via SDIO (ok, I think it's actually a Wi2Wi W2CBW003 module containing an 8686).

Up to the 3.1 mainline kernel WiFi worked just fine! I could boot the kernel and load the libertas module successfully:

(during boot)
[    3.193176] mmc1: new SDIO card at address 0001

(when loading module)
# modprobe libertas_sdio
[   30.817749] lib80211: common routines for IEEE802.11 drivers
[   31.143951] cfg80211: Calling CRDA to update world regulatory domain
[   31.375823] libertas_sdio: Libertas SDIO driver
[   31.380737] libertas_sdio: Copyright Pierre Ossman
[   32.035186] libertas_sdio mmc1:0001:1: (unregistered net_device): 00:19:88:3e:4f:f9, fw 9.70.3p36, cap 0x00000303
[   32.129943] libertas_sdio mmc1:0001:1: wlan0: Marvell WLAN 802.11 adapter


However it seems that runtime PM has now been turned on by default (by setting MMC_CAP_POWER_OFF_CARD in drivers/mmc/core/host.c), and the WiFi 
has stopped working!

So, if I run the 3.2-rc2 (or rc1 for that matter) I get the following:

(during boot)
[    3.486541] mmc1: new SDIO card at address 0001

(when loading module)
# modprobe libertas_sdio
[   34.015350] lib80211: common routines for IEEE802.11 drivers
[   34.345092] cfg80211: Calling CRDA to update world regulatory domain
[   34.574096] libertas_sdio: Libertas SDIO driver
[   34.578979] libertas_sdio: Copyright Pierre Ossman
[   34.637481] libertas_sdio: probe of mmc1:0001:1 failed with error -16


Looking at arch/arm/mach-omap2/overo.c you can see that GPIO16 (OVERO_GPIO_W2W_NRESET) is the nRESET line for the 8686. This is held low for 
10usecs during boot (see overo.c line 530 to 533).

I'm not 100% sure how power is connected to the SD8686 as GUMSTIX don't release schematics for their modules.

If I disable runtime PM (by commenting out the setting of MMC_CAP_POWER_OFF_CARD) everything works as before.

I can see plenty of threads relating to getting the SD8686 working for OLPC, and that some workarounds have made it in to the mainline kernel. From what 
I understand the problem is that runtime PM removes power from the 8686 after boot and before the loading of the libertas module.

Are these workarounds expected to be enough for runtime PM to just "work"?
Is there anything special I should be doing with the nRESET line other than that which is done by default in the board file (holding nRESET low for 10usecs)?

Cheers,
Joe




^ permalink raw reply	[flat|nested] 26+ messages in thread
* RE: [PATCH 2/2] mmc: core: Support packed command for eMMC4.5 device
@ 2011-11-11  7:26 Seungwon Jeon
  2011-11-11  9:38 ` S, Venkatraman
  0 siblings, 1 reply; 26+ messages in thread
From: Seungwon Jeon @ 2011-11-11  7:26 UTC (permalink / raw)
  To: merez
  Cc: 'S, Venkatraman', linux-mmc, 'Chris Ball',
	linux-kernel, linux-samsung-soc, kgene.kim, dh.han

Maya Erez wrote:
> On Thu, Nov 10, 2011 Maya Erez wrote:
> > S, Venkatraman <svenkatr@ti.com> wrote:
> >> On Thu, Nov 3, 2011 at 7:23 AM, Seungwon Jeon <tgih.jun@samsung.com>
> wrote:
> >> >> > +static u8 mmc_blk_chk_packable(struct mmc_queue *mq, struct
> >> request *req)
> 
> The function prepares the checkable list and not only checks if packing is
> possible, therefore I think its name should change to reflect its real
> action
I labored at naming. Isn't it proper? :)
Do you have any recommendation?
group_pack_req?

> 
> >> >> > +       if (!(md->flags & MMC_BLK_CMD23) &&
> >> >> > +                       !card->ext_csd.packed_event_en)
> >> >> > +               goto no_packed;
> 
> Having the condition with a && can lead to cases where CMD23 is not
> supported and we send packed commands. Therfore the condition should be
> changed to || or be splitted to 2 separate checks.
> Also, according to the standard the generic error flag in
> PACKED_COMMAND_STATUS is set in case of any error and having
> PACKED_EVENT_EN is only optional. Therefore, I don't think that setting
> the packed_event_en should be a mandatory condition for using packed
> coammnds.
... cases where CMD23 is not supported and we send packed commands?
Packed command must not be allowed in such a case.
It works only with predefined mode which is essential fator.
And spec doesn't mentioned PACKED_EVENT_EN must be set.
So Packed command can be sent regardless PACKED_EVENT_EN,
but it's not complete without reporting of error.
Then host driver may suffer error recovery.
Why packed command is used without error reporting?

> 
> >> >> > +       if (mmc_req_rel_wr(cur) &&
> >> >> > +                       (md->flags & MMC_BLK_REL_WR) &&
> >> >> > +                       !en_rel_wr) {
> >> >> > +               goto no_packed;
> >> >> > +       }
> 
> Can you please explain this condition and its purpose?
> 
In the case where reliable write is request but enhanced reliable write
is not supported, write access must be partial according to
reliable write sector count. Because even a single request can be split,
packed command is not allowed in this case.

> >> >> > +               phys_segments +=  next->nr_phys_segments;
> >> >> > +               if (phys_segments > max_phys_segs) {
> >> >> > +                       blk_requeue_request(q, next);
> >> >> > +                       break;
> >> >> > +               }
> >> >> I mentioned this before - if the next request is not packable and
> >> requeued,
> >> >> blk_fetch_request will retrieve it again and this while loop will
> never terminate.
> >> >>
> >> > If next request is not packable, it is requeued and 'break'
> terminates
> >> this loop.
> >> > So it not infinite.
> >> Right !! But that doesn't help finding the commands that are packable.
> Ideally, you'd need to pack all neighbouring requests into one packed
> command.
> >> The way CFQ works, it is not necessary that the fetch would return all
> outstanding
> >> requests that are packable (unless you invoke a forced dispatch) It
> would be good to see some numbers on the number of pack hits /
> misses
> >> that
> >> you would encounter with this logic, on a typical usecase.
> > Is it considered only for CFQ scheduler? How about other I/O scheduler?
> If all requests are drained from scheduler queue forcedly,
> > the number of candidate to be packed can be increased.
> > However we may lose the unique CFQ's strength and MMC D/D may take the
> CFQ's duty.
> > Basically, this patch accommodates the origin order requests from I/O
> scheduler.
> >
> 
> In order to better utilize the packed commands feature and achieve the
> best performance improvements I think that the command packing should be
> done in the block layer, according to the scheduler policy.
> That is, the scheduler should be aware of the capability of the device to
> receive a request list and its constrains (such as maximum number of
> requests, max number of sectors etc) and use this information as a  factor
> to its algorithm.
> This way you keep the decision making in the hands of the scheduler while
> the MMC layer will only have to send this list as a packed command.
> 
Yes, it would be another interesting approach.
Command packing you mentioned means gathering request among same direction(read/write)?
Currently I/O scheduler may know device constrains which MMC driver informs
with the exception of order information for packed command.
But I think the dependency of I/O scheduler may be able to come up.
How can MMC layer treat packed command with I/O scheduler which doesn't support this?

> >> >> > +       if (rqc)
> >> >> > +               reqs = mmc_blk_chk_packable(mq, rqc);
> 
> It would be best to keep all the calls to blk_fetch_request in the same
> location. Therefore, I suggest to move the call to mmc_blk_chk_packable to
> mmc/card/queue.c after the first request is fetched.

At the first time, I considered that way.
I'll do more, if possible.
> 
> >> >> >  cmd_abort:
> >> >> > -       spin_lock_irq(&md->lock);
> >> >> > -       while (ret)
> >> >> > -               ret = __blk_end_request(req, -EIO,
> >> blk_rq_cur_bytes(req));
> >> >> > -       spin_unlock_irq(&md->lock);
> >> >> > +       if (mq_rq->packed_cmd != MMC_PACKED_NONE) {
> 
> This should be the case for MMC_PACKED_NONE.
Right, I missed it.
> 
> >> >> > +               spin_lock_irq(&md->lock);
> >> >> > +               while (ret)
> >> >> > +                       ret = __blk_end_request(req, -EIO,
> >> blk_rq_cur_bytes(req));
> 
> Do we need the while or should it be an if? In other cases where
> __blk_end_request is called there is no such while.
This part is not only the new but also origin code which is moved in this patch.
Maybe...,'If' case is used  for a whole of remained bytes and
'while' case is used for partial report of remained bytes.

Thank you for review.

Best regards,
Seugwon Jeon.
> 
> Thanks,
> Maya Erez
> Consultant for Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
> 
> 
> 
> 
> --
> 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

^ permalink raw reply	[flat|nested] 26+ messages in thread
end of thread, other threads:[~2012-04-28  9:51 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-01 11:29 mmc: sdio: runtime PM and 8686 problems Joe Woodward
  -- strict thread matches above, loose matches on Subject: below --
2011-12-01 11:29 Joe Woodward
2011-11-18  8:53 Joe Woodward
2011-11-18  9:06 ` Ohad Ben-Cohen
2011-11-18 10:00   ` Joe Woodward
2011-11-18 12:15     ` Ohad Ben-Cohen
2011-11-29 11:17       ` Joe Woodward
2011-11-29 12:43         ` Ohad Ben-Cohen
2011-11-29 21:42           ` Daniel Drake
2011-11-30 10:36             ` Ohad Ben-Cohen
2011-11-30 14:12               ` Daniel Drake
2011-11-30 14:29                 ` Ohad Ben-Cohen
2011-11-30 14:57                   ` Daniel Drake
2011-12-01  7:50                     ` Ohad Ben-Cohen
2011-11-30 15:00                   ` Joe Woodward
2011-12-01  8:07                     ` Ohad Ben-Cohen
2011-12-01 11:28                       ` Joe Woodward
2011-12-03 16:44                         ` Daniel Drake
2011-12-03 16:45                           ` Daniel Drake
2011-12-07 12:28                           ` Joe Woodward
2011-12-19  9:10                   ` Ohad Ben-Cohen
2011-12-19  9:22                     ` Ohad Ben-Cohen
2012-04-26 23:51                     ` Steve Sakoman
2012-04-27  8:26                       ` Joe Woodward
2012-04-28  9:51                       ` NeilBrown
2011-11-11  7:26 [PATCH 2/2] mmc: core: Support packed command for eMMC4.5 device Seungwon Jeon
2011-11-11  9:38 ` S, Venkatraman
2011-11-11 19:01   ` merez
2011-11-14  9:46     ` Seungwon Jeon
2011-11-15 12:48       ` merez
2011-11-17  2:02         ` Seungwon Jeon
2011-11-17  9:16           ` mmc: sdio: runtime PM and 8686 problems Joe Woodward

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox