Hi Ulf, Sorry for the delay in response...
> -----Original Message----- > From: Ulf Hansson [mailto:ulf.hans...@linaro.org] > Sent: Tuesday, March 17, 2015 12:44 PM > To: Alex Lemberg > Cc: Avi Shchislowski; linux-mmc; Chris Ball > Subject: Re: [PATCH] mmc: sleep notification > > On 16 March 2015 at 17:58, Alex Lemberg <alex.lemb...@sandisk.com> > wrote: > > > > > >> -----Original Message----- > >> From: Ulf Hansson [mailto:ulf.hans...@linaro.org] > >> Sent: Monday, March 16, 2015 3:35 PM > >> To: Alex Lemberg > >> Cc: Avi Shchislowski; linux-mmc; Chris Ball > >> Subject: Re: [PATCH] mmc: sleep notification > >> > >> On 16 March 2015 at 12:37, Alex Lemberg <alex.lemb...@sandisk.com> > >> wrote: > >> > Hi Ulf, > >> > > >> >> -----Original Message----- > >> >> From: Ulf Hansson [mailto:ulf.hans...@linaro.org] > >> >> Sent: Thursday, March 12, 2015 11:10 AM > >> >> To: Alex Lemberg > >> >> Cc: Avi Shchislowski; linux-mmc; Chris Ball > >> >> Subject: Re: [PATCH] mmc: sleep notification > >> >> > >> >> [...] > >> >> > >> >> >> > Also, I think we need to clarify one more point for this patch: > >> >> >> > As was mentioned in commit message - Sleep_Notification can > >> >> >> > be interrupted > >> >> >> by HPI. > >> >> >> > This allows not blocking the host during the > >> >> >> > Sleep_Notification busy time and allows accepting requests coming > during this stage. > >> >> >> > Thus, without having HPI supported, suspend/resume process > >> >> >> > might be influenced by Sleep_Notification busy time, and this > >> >> >> > should not happen - > >> >> >> suspend/resume should be done in very fast and not blocking manner. > >> >> >> > >> >> >> I fail to understand your comment here. > >> >> >> > >> >> >> Please tell me at what point(s) your think it make sense to > >> >> >> issue the SLEEP_NOTIFICATION? If that is during the suspend > >> >> >> phase, then a HPI request can't be triggered. > >> >> > > >> >> > I think SLEEP_NOTIFICATION should be issued on mmc_pm_notify() > >> >> > call, on PM_SUSPEND_PREPARE case. > >> >> > >> >> So, exactly why is that to prefer, comparing doing it in system PM > >> >> ->suspend() callback? > >> > > >> > Assuming that SLEEP_NOTIFICATION may take time (defined in > >> > SLEEP_NOTIFICATION_TIME byte in EXT_CSD [216]), I think it is > >> > better to send it from pm notifier - mmc_pm_notify(). > >> > >> I assume you think that you will "earn" some milliseconds doing it in > >> this phase, but I am not so sure. > > > > I was thinking mainly about how to prevent "blocking" during > Sleep_Notification process. > > > >> > >> After the PM_SUSPEND_PREPARE notifier, we still have the mmc block > >> queue open and are ready to serve requests. Therefore I would expect > >> the SLEEP_NOTIFICATION to potentially be interrupted by using HPI. > > > > Right, and this was a reason for adding HPI support during the > Sleep_Notification. > > > >> > >> Then we end up with the following sequence during the system PM sleep > phase. > >> 1. Issue SLEEP_NOTIFICATION. > >> 2. Interrupt SLEEP_NOTIFICATION, using HPI. > >> 3. Serve blk request > >> 4. Issue SLEEP_NOTIFICATION. > >> 5. Issue SLEEP (CMD5). > > > > Correct, this is a sequence that we see after adding the patch. > > > >> That seems like a bad idea to me. > > > > Could you please explain why? > > Would you suggest to call Sleep_Notification from Suspend only? > > Since otherwise we may end up issuing SLEEP_NOTIFICATION not only once, but > twice during a system PM sleep sequence. So then we actually get an increased > system PM suspend time. > > > What if Sleep_Notification will take several seconds? > > Yes, that horrible! You should tell your colleagues designing FTLs to make > sure > that _never_ happens. Agree, but we need to consider and take care of all such cases in the driver side. Device might be busy with its internal garbage collection, and as spec allows, it will complete it in a great manner after host sends PON command. > > Also, this makes me wonder how this kind of feature ever could make it into > the > JEDEC specification. > > What we had earlier with "CMD5 only" should never have been changed. > > Kind regards > Uffe
