from:"Sujit Reddy Thumma"

[PATCH 1/5] mmc: core: Add support to read command queue parameters

2014-12-02 Thread Sujit Reddy Thumma

eMMC cards with EXT_CSD version >= 7, optionally support command
queuing feature as defined by Jedec eMMC5.1. Add support for probing
command queue feature for such type of cards.

Signed-off-by: Sujit Reddy Thumma 
Signed-off-by: Asutosh Das 
---
 drivers/mmc/core/mmc.c   | 15 +++
 include/linux/mmc/card.h |  7 +++
 include/linux/mmc/mmc.h  |  6 ++
 3 files changed, 28 insertions(+)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 1ab5f3a..0ef3af5 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -571,6 +571,21 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 
*ext_csd)
card->ext_csd.data_sector_size = 512;
}
 
+   if (card->ext_csd.rev >= 7) {
+   card->ext_csd.cmdq_support = ext_csd[EXT_CSD_CMDQ_SUPPORT];
+   if (card->ext_csd.cmdq_support)
+   card->ext_csd.cmdq_depth = ext_csd[EXT_CSD_CMDQ_DEPTH];
+   }
+
+   if (card->ext_csd.rev >= 7) {
+   card->ext_csd.cmdq_support = ext_csd[EXT_CSD_CMDQ_SUPPORT];
+   if (card->ext_csd.cmdq_support)
+   card->ext_csd.cmdq_depth = ext_csd[EXT_CSD_CMDQ_DEPTH];
+   } else {
+   card->ext_csd.cmdq_support = 0;
+   card->ext_csd.cmdq_depth = 0;
+   }
+
 out:
return err;
 }
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index b730272..b87a27c 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -112,6 +112,9 @@ struct mmc_ext_csd {
u8  raw_pwr_cl_ddr_52_360;  /* 239 */
u8  raw_bkops_status;   /* 246 */
u8  raw_sectors[4]; /* 212 - 4 bytes */
+   u8  cmdq_mode_en;   /* 15 */
+   u8  cmdq_depth; /* 307 */
+   u8  cmdq_support;   /* 308 */
 
unsigned intfeature_support;
 #define MMC_DISCARD_FEATUREBIT(0)  /* CMD38 feature */
@@ -259,6 +262,7 @@ struct mmc_card {
 #define MMC_STATE_HIGHSPEED_200(1<<8)  /* card is in HS200 
mode */
 #define MMC_STATE_DOING_BKOPS  (1<<10) /* card is doing BKOPS */
 #define MMC_STATE_SUSPENDED(1<<11) /* card is suspended */
+#define MMC_STATE_CMDQ   (1<<12) /* card is in cmd queue mode */
unsigned intquirks; /* card quirks */
 #define MMC_QUIRK_LENIENT_FN0  (1<<0)  /* allow SDIO FN0 writes 
outside of the VS CCCR range */
 #define MMC_QUIRK_BLKSZ_FOR_BYTE_MODE (1<<1)   /* use func->cur_blksize */
@@ -427,6 +431,7 @@ static inline void __maybe_unused remove_quirk(struct 
mmc_card *card, int data)
 #define mmc_card_removed(c)((c) && ((c)->state & MMC_CARD_REMOVED))
 #define mmc_card_doing_bkops(c)((c)->state & MMC_STATE_DOING_BKOPS)
 #define mmc_card_suspended(c)  ((c)->state & MMC_STATE_SUSPENDED)
+#define mmc_card_cmdq(c)   ((c)->state & MMC_STATE_CMDQ)
 
 #define mmc_card_set_present(c)((c)->state |= MMC_STATE_PRESENT)
 #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
@@ -441,6 +446,8 @@ static inline void __maybe_unused remove_quirk(struct 
mmc_card *card, int data)
 #define mmc_card_clr_doing_bkops(c)((c)->state &= ~MMC_STATE_DOING_BKOPS)
 #define mmc_card_set_suspended(c) ((c)->state |= MMC_STATE_SUSPENDED)
 #define mmc_card_clr_suspended(c) ((c)->state &= ~MMC_STATE_SUSPENDED)
+#define mmc_card_set_cmdq(c)   ((c)->state |= MMC_STATE_CMDQ)
+#define mmc_card_clr_cmdq(c)   ((c)->state &= ~MMC_STATE_CMDQ)
 
 /*
  * Quirk add/remove for MMC products.
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 50bcde3..a893c84 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -84,6 +84,9 @@
 #define MMC_APP_CMD  55   /* ac   [31:16] RCAR1  */
 #define MMC_GEN_CMD  56   /* adtc [0] RD/WR  R1  */
 
+/* MMC 5.1 - class 11: Command Queueing */
+#define MMC_CMDQ_TASK_MGMT48  /* ac   [31:0] task ID R1b */
+
 static inline bool mmc_op_multi(u32 opcode)
 {
return opcode == MMC_WRITE_MULTIPLE_BLOCK ||
@@ -272,6 +275,7 @@ struct _mmc_csd {
  * EXT_CSD fields
  */
 
+#define EXT_CSD_CMDQ_MODE  15  /* R/W */
 #define EXT_CSD_FLUSH_CACHE32  /* W */
 #define EXT_CSD_CACHE_CTRL 33  /* R/W */
 #define EXT_CSD_POWER_OFF_NOTIFICATION 34  /* R/W */
@@ -325,6 +329,8 @@ struct _mmc_csd {
 #define EXT_CSD_POWER_OFF_LONG_TIME247 /* RO */
 #define EXT_CSD_GENERIC_CMD6_TIME  248 /* RO */
 #define EXT_CSD_CACHE_SIZE 249 /* RO, 4 bytes */
+#define EXT_CSD_CMDQ_DEPTH 307 /* RO */
+#define EXT_CSD_CMDQ_SUPPORT

Re: [PATCH 0/3] mmc: Use runtime pm for blkdevice

2013-03-27 Thread Sujit Reddy Thumma


Hi Ulf,

Sorry for the delayed response.
The patches looks good to me except one concern mentioned below.

On 3/21/2013 3:28 AM, Ulf Hansson wrote:



If any driver wants to implement this then the runtime PM code would be
refactored again. So I guess we might want to think about this now itself?


Refactored, no.

It is just a new feature that needs to be added, should be a rather
simple patch. Since this kind of code has not been upstreamed for your
host driver I did not consider it in this initial step. Do you want me
to create an additional patch on top of this patchset? I can help out
if you like.



What about SD cards? For SD cards the runtime PM is not doing any advantage
but instead waste cpu cycles with a timer interrupt and running noop runtime
PM callbacks? I guess allowing to power off cards in such cases would have
decent power savings.


We will waste some cpu cycles, true.

Do you think that will have bad impact on performance? In that case
why do we even bother doing runtime PM in host drivers and in many
other places in the kernel? Of course we could optmize the code and
only enable runtime PM if there are a corresponding runtime pm
callbacks implemented in the bus_ops, but is it needed?


Well.. my point here is that runtime PM framework unnecessarily wakeup 
processor (if idle) every "x" secs without doing any useful work. If 
that is agreeable then I am okay with not having the optimization.









Please have a look at below thread to find the answers to your questions:
http://thread.gmane.org/gmane.linux.kernel.mmc/19444/focus=19443



Thanks a lot. I have missed this discussion :(
I have some comments on the possible solutions:

"In mmc bus_ops runtime callback, do a pm_runtime_get_sync("host plf
device"), and vice verse in the runtime resume callback. This will
prevent the host driver from entering runtime power save sate while
for example doing BKOPS, thus preventing your host driver from doing
mmc_suspend_host while BKOPS is running"

[Sujit] In addition, probably we can allow host to turn off the clocks while
carrying out BKOPS. But, how can we know whether card is done with BKOPS and
is idle so that we can call mmc_suspend_host()?


We are then going into details about how to implement IDLE BKOPS,
which is a bit out of scope for this patch, but let me try to comment
anyway.

The initial patch for BKOPS could skip your consideration, and just
check for BKOPS complete once runtime suspend callback gets called.
This will then be rather simple to implement and work for all cases
but yours. I realize that a new blk request will be needed to move out
from BKOPS state then.

The next step could be to schedule a timer/work when issuing BKOPS,
that polls for completion. I belive it should be rather simple to
extend the runtime pm callbacks with this support, right?


Thanks for the details. It looks clear to me now.





"Move mmc_suspend|resume_host from your host runtime callbacks, into
the bus_ops runtime callbacks. Typically, when no BKOPS is needed
mmc_suspend_host can be done."

[Sujit] Doesn't it look like we are establishing parent child relationship
here? If the card has nothing to do, suspend the host?


Well, the naming of these functions are not correct. It is not the
host that actually gets suspended, it is the card.

Right now these functions happens to be called when a host enters
suspend though, which indeed is also kind of strange. It would make
more sense to handle card suspend from the mmc/sdio bus instead; but
let's leave that for a separate discussion. :-)


I agree.



I also assume that if your host driver runtime pm callbacks calls
mmc_suspend|resume_host, your host driver system suspend|resume
callbacks must not - otherwise you will have races! Instead upper
layers like a power domain, must force your device into runtime
suspend when entering system suspend and vice verse when doing system
resume. These issues exists with or without my patches.



Possibly, the races can be avoided using pm_runtime_suspended() check, 
but I am not sure if it is the clean way.



--
Regards,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] mmc: Use runtime pm for blkdevice

2013-03-20 Thread Sujit Reddy Thumma


According the eMMC spec I can't see any requirement that the bus clock
needs to be on while a BKOPS is running. Moreover it is clearly stated
it is allowed to gate the bus clock for a device which indicates busy.
So, I can't see that this is needed.



What if host is aggressive and wants to keep eMMC in sleep-mode and turn off
VCC regulator during runtime power management? I believe that eMMC card
needs VCC supply as well in addition to VCCQ to carry out BKOPS. Do you
think that the block device also needs to take a reference for VCC regulator
while BKOPS is started in runtime suspend of block device?


What you are thinking of would be exactly the same scenario as doing
"mmc_suspend_host" from a host runtime suspend callback, which have
been discussed here earlier. Right now, no up-streamed host driver is
doing this and I would guess it would never happen either. Anyway,
still worth to consider somehow.


If any driver wants to implement this then the runtime PM code would be 
refactored again. So I guess we might want to think about this now itself?


What about SD cards? For SD cards the runtime PM is not doing any 
advantage but instead waste cpu cycles with a timer interrupt and 
running noop runtime PM callbacks? I guess allowing to power off cards 
in such cases would have decent power savings.




Please have a look at below thread to find the answers to your questions:
http://thread.gmane.org/gmane.linux.kernel.mmc/19444/focus=19443



Thanks a lot. I have missed this discussion :(
I have some comments on the possible solutions:

"In mmc bus_ops runtime callback, do a pm_runtime_get_sync("host plf
device"), and vice verse in the runtime resume callback. This will
prevent the host driver from entering runtime power save sate while
for example doing BKOPS, thus preventing your host driver from doing
mmc_suspend_host while BKOPS is running"

[Sujit] In addition, probably we can allow host to turn off the clocks 
while carrying out BKOPS. But, how can we know whether card is done with 
BKOPS and is idle so that we can call mmc_suspend_host()?


"Move mmc_suspend|resume_host from your host runtime callbacks, into
the bus_ops runtime callbacks. Typically, when no BKOPS is needed
mmc_suspend_host can be done."

[Sujit] Doesn't it look like we are establishing parent child 
relationship here? If the card has nothing to do, suspend the host?



--
Regards,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] mmc: Use runtime pm for blkdevice

2013-03-14 Thread Sujit Reddy Thumma


On 3/8/2013 8:44 AM, Ulf Hansson wrote:

On 7 March 2013 22:14, Kevin Liu  wrote:

2013/3/7 Kevin Liu :

2013/3/7 Ulf Hansson :

On 7 March 2013 01:12, Kevin Liu  wrote:

From: Ulf Hansson 
mailto:ulf.hans...@stericsson.com>>
Date: Fri, Mar 1, 2013 at 8:47 PM
Subject: [PATCH 0/3] mmc: Use runtime pm for blkdevice
To: linux-mmc@vger.kernel.org, Chris Ball 
mailto:c...@laptop.org>>
Cc: Johan Rudholm mailto:johan.rudh...@stericsson.com>>, Ulf 
Hansson mailto:ulf.hans...@linaro.org>>


From: Ulf Hansson mailto:ulf.hans...@linaro.org>>

SDIO has been using runtime pm for a while to handle runtime power save
operations. This patchset is enabling the option to make the sd/mmc
blockdevices to use runtime pm as well.

The runtime pm implementation for the block device will make use of
autosuspend to defer power save operation to after request inactivty for
a certain time.

To actually perform some power save operations the corresponding bus ops
for mmc and sd shall be implemented. Typically it could make sense to do
BKOPS for eMMC in here.

Ulf Hansson (3):
   mmc: core: Remove power_restore bus_ops for mmc and sd
   mmc: core: Add bus_ops for runtime pm callbacks
   mmc: block: Enable runtime pm for mmc blkdevice


Ulf,

sdhci.c has added pm_runtime which also protect between request and
task finish. And some sdhci.c based host drivers has provided
pm_runtime_suspend/resume functions like sdhci-pxav3.c. From the
powersave viewpoint, I think adding pm_runtime in driver level is
better than doing that on bus level since the control granularity is
even smaller. And adding pm_runtime in both block.c and sdhci.c will
call pm_runtime twice. How do you think?

Thanks
Kevin


Hi Kevin,

Thanks for your response!

It seems like we need some more clarification around this area.
Runtime pm for a host device driver shall ultimately be responsible
for taking care of runtime power management of the host device - only.
It should not handle runtime power management of a block device, which
in principle means BKOPS shall be handled in the blkdevice. At least
this is my view.

So, why is this? I will try to elaborate on the runtime pm support in
host drivers here.
The host device driver controls a MMC/SD/SDIO IP. This IP could very
well reside (for some SoC) in what you call a power domain. In
principle, once the IP needs to be used, a host driver has done a
pm_runtime_get of it's device. This will mean a reference to the power
domain has been fetched. Once the IP is not needed any more,
pm_runtime_put is done and the reference to the power domain is
released. Once no reference to the power domain exist the power domain
can enter lower sleep states, which is preferred to happen as soon as
possible and as long as possible - of course.

Hope this gives a better understanding. :-)


Ulf,

Thanks for the explanations!
Then do you mean to start bkops when blkdev pm_runtime auto suspended
while stop bkops when blkdev pm_runtime resumed?
My only concern is that we have implemented pm_runtime for host device
and its pm_runtime functions will turn on/off bus clock when host dev
runtime resume/suspend. Let's see below sequence when an issue request
come:
1. blkdev pm_runtime resumed in mmc_blk_issue_rq.
2. blkdev issue request
3. host dev pm_runtime resumed in host->ops->request.
4. host finished the transfer and host dev pm_runtime suspended.
5. 3s later, blkdev pm_runtime suspended.
The bus clock will be turn off in step 4 by host dev
pm_runtime_suspend function. Then how can bkops run in step 5?


My question is host dev will stop bus clock by pm_runtime_suspend once
the request transfer is finished. But bkops on emmc chip should still
need the bus clock after bkops started. How to handle this?


According the eMMC spec I can't see any requirement that the bus clock
needs to be on while a BKOPS is running. Moreover it is clearly stated
it is allowed to gate the bus clock for a device which indicates busy.
So, I can't see that this is needed.



What if host is aggressive and wants to keep eMMC in sleep-mode and turn 
off VCC regulator during runtime power management? I believe that eMMC 
card needs VCC supply as well in addition to VCCQ to carry out BKOPS. Do 
you think that the block device also needs to take a reference for VCC 
regulator while BKOPS is started in runtime suspend of block device?



--
Regards,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: core: Do not pre-claim host in suspend

2012-04-20 Thread Sujit Reddy Thumma

mc_claim_host(host);
> - mmc_detach_bus(host);
> - mmc_power_off(host);
> - mmc_release_host(host);
> - host->pm_flags = 0;
> - err = 0;
> - }
> - } else {
> - err = -EBUSY;
> + if (err == -ENOSYS || !host->bus_ops->resume) {
> + /*
> +  * We simply "remove" the card in this case.
> +  * It will be redetected on resume.  (Calling
> +  * bus_ops->remove() with a claimed host can
> +  * deadlock.)
> +  */
> + if (host->bus_ops->remove)
> + host->bus_ops->remove(host);
> + mmc_claim_host(host);
> +     mmc_detach_bus(host);
> + mmc_power_off(host);
> + mmc_release_host(host);
> + host->pm_flags = 0;
> + err = 0;
>   }
>   }
>   mmc_bus_put(host);
> --
> 1.7.9
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Thanks & Regards,
Sujit Reddy Thumma

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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: sdio: avoid spurious calls to interrupt handlers

2012-04-15 Thread Sujit Reddy Thumma


> On Sun, 15 Apr 2012, Sujit Reddy Thumma wrote:
>
>> Hi Nicolas,
>>
>> >
>> > Commit 06e8935feb "optimized SDIO IRQ handling for single irq"
>> > introduced some spurious calls to SDIO function interrupt handlers,
>> > such as when the SDIO IRQ thread is started, or the safety check
>> > performed upon a system resume.  Let's add a flag to perform the
>> > optimization only when a real interrupt is signaled by the host
>> > driver and we know there is no point confirming it.
>> >
>>
>> Thanks for putting up formal patch.
>>
>> > Reported-by: Sujit Reddy Thumma 
>> > Signed-off-by: Nicolas Pitre 
>> > Cc: sta...@kernel.org
>> >
>> > diff --git a/drivers/mmc/core/sdio_irq.c b/drivers/mmc/core/sdio_irq.c
>> > index f573e7f9f7..3d8ceb4084 100644
>> > --- a/drivers/mmc/core/sdio_irq.c
>> > +++ b/drivers/mmc/core/sdio_irq.c
>> > @@ -28,18 +28,20 @@
>> >
>> >  #include "sdio_ops.h"
>> >
>> > -static int process_sdio_pending_irqs(struct mmc_card *card)
>> > +static int process_sdio_pending_irqs(struct mmc_host *host)
>> >  {
>> > +  struct mmc_card *card = host->card;
>> >int i, ret, count;
>> >unsigned char pending;
>> >struct sdio_func *func;
>> >
>> >/*
>> > * Optimization, if there is only 1 function interrupt registered
>> > -   * call irq handler directly
>> > +   * and we know an IRQ was signaled then call irq handler directly.
>> > +   * Otherwise do the full probe.
>> > */
>> >func = card->sdio_single_irq;
>> > -  if (func) {
>> > +  if (func && host->sdio_irq_pending) {
>> >func->irq_handler(func);
>> >return 1;
>> >}
>> > @@ -116,7 +118,8 @@ static int sdio_irq_thread(void *_host)
>> >ret = __mmc_claim_host(host, &host->sdio_irq_thread_abort);
>> >if (ret)
>> >break;
>> > -  ret = process_sdio_pending_irqs(host->card);
>> > +  ret = process_sdio_pending_irqs(host);
>> > +  host->sdio_irq_pending = false;
>> >mmc_release_host(host);
>> >
>> >/*
>> > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> > index ee2b0363c0..557aa4cd66 100644
>> > --- a/include/linux/mmc/host.h
>> > +++ b/include/linux/mmc/host.h
>> > @@ -323,6 +323,7 @@ struct mmc_host {
>> >
>> >unsigned intsdio_irqs;
>> >struct task_struct  *sdio_irq_thread;
>> > +  boolsdio_irq_pending;
>> >atomic_tsdio_irq_thread_abort;
>> >
>> >mmc_pm_flag_t   pm_flags;   /* requested pm features */
>> > @@ -378,6 +379,7 @@ extern int mmc_cache_ctrl(struct mmc_host *, u8);
>> >  static inline void mmc_signal_sdio_irq(struct mmc_host *host)
>> >  {
>> >host->ops->enable_sdio_irq(host, 0);
>> > +  host->sdio_irq_pending = true;
>> >wake_up_process(host->sdio_irq_thread);
>> >  }
>>
>> In this case probably we need to add the following:
>> @@ -946,8 +946,11 @@ static int mmc_sdio_resume(struct mmc_host *host)
>> }
>> }
>>
>> -   if (!err && host->sdio_irqs)
>> -   mmc_signal_sdio_irq(host);
>> +   if (!err && host->sdio_irqs) {
>> +   host->ops->enable_sdio_irq(host, 0);
>> +   wake_up_process(host->sdio_irq_thread);
>> +   }
>
> The call to enable_sdio_irq() is probably redundant.  Only
> wake_up_process() should be sufficient.
>

True, I was wondering if we really need to wakeup sdio_irq_thread here. If
there is a pending interrupt is it not the host driver supposed to wake it
up or do you think it is needed for hosts that don't have CAP_SDIO_IRQ
set?

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: sdio: avoid spurious calls to interrupt handlers

2012-04-15 Thread Sujit Reddy Thumma

Hi Nicolas,

>
> Commit 06e8935feb "optimized SDIO IRQ handling for single irq"
> introduced some spurious calls to SDIO function interrupt handlers,
> such as when the SDIO IRQ thread is started, or the safety check
> performed upon a system resume.  Let's add a flag to perform the
> optimization only when a real interrupt is signaled by the host
> driver and we know there is no point confirming it.
>

Thanks for putting up formal patch.

> Reported-by: Sujit Reddy Thumma 
> Signed-off-by: Nicolas Pitre 
> Cc: sta...@kernel.org
>
> diff --git a/drivers/mmc/core/sdio_irq.c b/drivers/mmc/core/sdio_irq.c
> index f573e7f9f7..3d8ceb4084 100644
> --- a/drivers/mmc/core/sdio_irq.c
> +++ b/drivers/mmc/core/sdio_irq.c
> @@ -28,18 +28,20 @@
>
>  #include "sdio_ops.h"
>
> -static int process_sdio_pending_irqs(struct mmc_card *card)
> +static int process_sdio_pending_irqs(struct mmc_host *host)
>  {
> + struct mmc_card *card = host->card;
>   int i, ret, count;
>   unsigned char pending;
>   struct sdio_func *func;
>
>   /*
>* Optimization, if there is only 1 function interrupt registered
> -  * call irq handler directly
> +  * and we know an IRQ was signaled then call irq handler directly.
> +  * Otherwise do the full probe.
>*/
>   func = card->sdio_single_irq;
> - if (func) {
> + if (func && host->sdio_irq_pending) {
>   func->irq_handler(func);
>   return 1;
>   }
> @@ -116,7 +118,8 @@ static int sdio_irq_thread(void *_host)
>   ret = __mmc_claim_host(host, &host->sdio_irq_thread_abort);
>   if (ret)
>   break;
> - ret = process_sdio_pending_irqs(host->card);
> + ret = process_sdio_pending_irqs(host);
> + host->sdio_irq_pending = false;
>   mmc_release_host(host);
>
>   /*
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index ee2b0363c0..557aa4cd66 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -323,6 +323,7 @@ struct mmc_host {
>
>   unsigned intsdio_irqs;
>   struct task_struct  *sdio_irq_thread;
> + boolsdio_irq_pending;
>   atomic_tsdio_irq_thread_abort;
>
>   mmc_pm_flag_t   pm_flags;   /* requested pm features */
> @@ -378,6 +379,7 @@ extern int mmc_cache_ctrl(struct mmc_host *, u8);
>  static inline void mmc_signal_sdio_irq(struct mmc_host *host)
>  {
>   host->ops->enable_sdio_irq(host, 0);
> + host->sdio_irq_pending = true;
>   wake_up_process(host->sdio_irq_thread);
>  }

In this case probably we need to add the following:
@@ -946,8 +946,11 @@ static int mmc_sdio_resume(struct mmc_host *host)
}
}

-   if (!err && host->sdio_irqs)
-   mmc_signal_sdio_irq(host);
+   if (!err && host->sdio_irqs) {
+   host->ops->enable_sdio_irq(host, 0);
+       wake_up_process(host->sdio_irq_thread);
+   }

>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Thanks & Regards,
Sujit Reddy Thumma

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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: BUG/PATCH? mmcblk devices don't suspend properly.

2012-03-29 Thread Sujit Reddy Thumma


On 3/30/2012 8:17 AM, NeilBrown wrote:


I've been experimenting with removing the call to sys_sync() in
enter_state().  For me (with verbose debugging and syslog running)
this causes a noticeable delay when entering suspend.

Removing it should not affect correctness as there is no locking the ensure
that no other process writes out data immediately after the 'sync'
completed.  But it could make existing bugs more obvious - in fact it does :-)

The device I am doing this on is using a micro-SD card for storage.
The card cannot be removed without removing the battery, so
MMC_CAP_NONREMOVABLE is set for the mmc slot (which seems necessary for
safely having '/' there).

Since removing the sys_sync() call I've notices a number of problems with
suspend/resume, the most obvious being that resume blocks in mmc_claim_host:

[  263.585754] susman  D c046455c  5604  1086   1084 0x
[  263.592498] [] (__schedule+0x584/0x614) from [] 
(__mmc_claim_host+0xb8/0x154)
[  263.601806] [] (__mmc_claim_host+0xb8/0x154) from [] 
(mmc_sd_resume+0x34/0x5c)
[  263.611206] [] (mmc_sd_resume+0x34/0x5c) from [] 
(mmc_resume_host+0xc8/0x15c)
[  263.620513] [] (mmc_resume_host+0xc8/0x15c) from [] 
(omap_hsmmc_resume+0xbc/0x104)
[  263.630279] [] (omap_hsmmc_resume+0xbc/0x104) from [] 
(platform_pm_resume+0x44/0x54)
[  263.640197] [] (platform_pm_resume+0x44/0x54) from [] 
(dpm_run_callback+0x48/0x8c)
[  263.649963] [] (dpm_run_callback+0x48/0x8c) from [] 
(device_resume+0x204/0x268)
[  263.659454] [] (device_resume+0x204/0x268) from [] 
(dpm_resume+0x10c/0x244)
[  263.668579] [] (dpm_resume+0x10c/0x244) from [] 
(dpm_resume_end+0xc/0x18)
[  263.677490] [] (dpm_resume_end+0xc/0x18) from [] 
(suspend_devices_and_enter+0x1d0/0x22c)
[  263.687805] [] (suspend_devices_and_enter+0x1d0/0x22c) from 
[] (enter_state+0x118/0x170)
[  263.698089] [] (enter_state+0x118/0x170) from [] 
(state_store+0x94/0x118)
[  263.707061] [] (state_store+0x94/0x118) from [] 
(kobj_attr_store+0x1c/0x24)
[  263.716186] [] (kobj_attr_store+0x1c/0x24) from [] 
(sysfs_write_file+0x108/0x13c)
[  263.725860] [] (sysfs_write_file+0x108/0x13c) from [] 
(vfs_write+0xac/0x180)
[  263.735076] [] (vfs_write+0xac/0x180) from [] 
(sys_write+0x40/0x6c)
[  263.743469] [] (sys_write+0x40/0x6c) from [] 
(ret_fast_syscall+0x0/0x3c)


while mmcdq/0 is owning the device and waiting for something that will
apparently never happen:

[  262.566680] mmcqd/0 D c046455c  582853  2 0x
[  262.573425] [] (__schedule+0x584/0x614) from [] 
(schedule_timeout+0x1c/0x1d0)
[  262.582733] [] (schedule_timeout+0x1c/0x1d0) from [] 
(wait_for_common+0xd8/0x150)
[  262.592407] [] (wait_for_common+0xd8/0x150) from [] 
(mmc_wait_for_req_done+0x24/0xbc)
[  262.602447] [] (mmc_wait_for_req_done+0x24/0xbc) from [] 
(mmc_start_req+0x50/0x11c)
[  262.612304] [] (mmc_start_req+0x50/0x11c) from [] 
(mmc_blk_issue_rw_rq+0x78/0x500)
[  262.622070] [] (mmc_blk_issue_rw_rq+0x78/0x500) from [] 
(mmc_blk_issue_rq+0x3f0/0x420)
[  262.632171] [] (mmc_blk_issue_rq+0x3f0/0x420) from [] 
(mmc_queue_thread+0x98/0x100)
[  262.642028] [] (mmc_queue_thread+0x98/0x100) from [] 
(kthread+0x84/0x90)
[  262.650878] [] (kthread+0x84/0x90) from [] 
(kernel_thread_exit+0x0/0x8)

I traced this to the fact that mmc_bus_suspend / mmc_bus_resume are *never*
being called.  So mmc_blk_suspend isn't called and the queue isn't suspended.

The problem appears to be that mmc_bus_suspend is defined as a 'legacy'
suspend function. i.e. it is assigned to mmc_bus_type.suspend rather than
mmc_bus_type.pm.suspend.
In __device_suspend() I see that the legacy suspend function is only called if
the bus has *no* dev_pm_ops at all.
However when CONFIG_PM_RUNTIME is defined, mmc_bustype does have a dev_pm_ops
which contains runtime_{suspend,resume,idle},but no suspend or resume.

The net effect is that - as I observed - mmc_bus_suspend is never called.

I added lines:

.suspend= mmc_bus_suspend,
.resume = mmc_bus_resume,

to mmc_bus_pm_ops and can no longer reproduce the problem.  So maybe this
patch is appropriate:

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 5d011a3..80c1e46 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -169,6 +169,8 @@ static const struct dev_pm_ops mmc_bus_pm_ops = {
.runtime_suspend= mmc_runtime_suspend,
.runtime_resume = mmc_runtime_resume,
.runtime_idle   = mmc_runtime_idle,
+   .suspend= mmc_bus_suspend,
+   .resume = mmc_bus_resume,
  };

  #define MMC_PM_OPS_PTR(&mmc_bus_pm_ops)

however I suspect we should remove the 'legacy' pointers at the same time.(?).
This was pointed out earlier and a patch is posted but looks like it 
never went into mmc tree -- 
http://comments.gmane.org/gmane.linux.kernel.mmc/9168




Also, while exploring this problem I could not find anything that would cause
mmc_bus_suspend

SDIO single irq optimization - Spurious interrupt notification to function driver

2012-03-29 Thread Sujit Reddy Thumma


Hi,

While I was debugging an issue where the sdio function driver panics 
upon receiving a spurious interrupt notification which is similar to the 
one found on libertas sdio 
http://permalink.gmane.org/gmane.linux.kernel.mmc/8338


I can confirm that my hardware not buggy and is not generating any 
interrupts during initialization and CCCR_INTx is all zero's.


After some analysis, I found that there is a notification of interrupt 
to function handler by sdio_irq_thread() even though there is no 
interrupt. The sequence is as follows.


sdio_claim_host();

sdio_claim_irq():
...
	sdio_card_irq_get() -> kthread_run(sdio_irq_thread) -> 
wake_up_sdio_irq_thread

sdio_single_irq_set() -> card->sdio_single_irq = func = sdio_func[i];
...

sdio_release_host();

Since we woke up sdio_irq_thread(), while this thread get a chance to run:
...
__mmc_claim_host()
process_sdio_pending_irqs() -> since sdio_single_irq is non NULL the 
func->handler is directly called without checking pending interrupt bit 
CCCR_INTx.

mmc_release_host()
...

Irrespective of the function driver state, sdio driver should not call 
func->handler() if it is not real interrupt. I knew that function 
handler should be able to handle these interrupts gracefully. But it is 
just not the right thing for sdio driver to generate spurious interrupts.


I see this notification issue during mmc_sdio_resume() also explained by 
Nicolas:

http://permalink.gmane.org/gmane.linux.kernel.mmc/10871

This leads me to think of reverting commit 06e8935feb "optimized SDIO 
IRQ handling for single irq" instead of hacking every function driver to 
handle spurious interrupt notification gracefully.


Let me know your thoughts on this or suggest any alternate way instead 
of reverting it.


--
Thanks & Regards,
Sujit Reddy Thumma
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: core: remove waiting time when clkgate_delay is set

2012-02-29 Thread Sujit Reddy Thumma


On 2/29/2012 11:22 AM, Chanho Park wrote:

Since recent commit("mmc: core: Use delayed work in clock gating
framework":597dd9d79cfbbb1), we always wait "unnecessary" default
clock delay(8 cycles). Actually, we don't need it if clkgate_delay
(unit:ms) is set because we already wait sufficient time to change
the clock due to delayed_workqueue.
This patch removes duplicated waiting time when clkgate_delay is set.

Signed-off-by: Chanho Park
Signed-off-by: Kyungmin Park
---
  drivers/mmc/core/host.c |7 +--
  1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index c3704e2..d710ce0 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -109,8 +109,11 @@ static void mmc_host_clk_gate_delayed(struct mmc_host 
*host)
 */
if (!host->clk_requests) {
spin_unlock_irqrestore(&host->clk_lock, flags);
-   tick_ns = DIV_ROUND_UP(10, freq);
-   ndelay(host->clk_delay * tick_ns);
+   /* wait only when clk_gate_delay is 0*/


Actually, we should check whether the clkgate_delay is giving sufficient 
clock cycles instead of just >0 value. But since min. f_min in freq. 
table is 100KHz (8CLK cyles is ~80us) and clkgate_delay can only 
specified in milliseconds as of now, I think this should be fine.



+   if (!host->clkgate_delay) {
+   tick_ns = DIV_ROUND_UP(10, freq);
+   ndelay(host->clk_delay * tick_ns);
+   }
} else {
/* New users appeared while waiting for this work */
spin_unlock_irqrestore(&host->clk_lock, flags);


Reviewed-by: Sujit Reddy Thumma 

Thanks,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: start removing enable / disable API

2012-02-28 Thread Sujit Reddy Thumma


Hi Adrian,

On 2/24/2012 4:19 PM, Adrian Hunter wrote:

Most parts of the enable / disable API are no longer used and
can be removed.

Cc: Rajendra Nayak
Cc: Venkatraman S
Cc: Kukjin Kim
Cc: Thomas Abraham
Cc: Kyungmin Park
Cc: Sekhar Nori
Cc: Kevin Hilman
Signed-off-by: Adrian Hunter
---
  arch/arm/mach-exynos/mach-nuri.c   |5 +-
  arch/arm/mach-exynos/mach-universal_c210.c |9 +-
  drivers/mmc/core/core.c|  185 ++--
  drivers/mmc/core/host.c|1 -
  drivers/mmc/core/host.h|1 -
  drivers/mmc/host/davinci_mmc.c |4 -
  drivers/mmc/host/omap_hsmmc.c  |   15 +--
  include/linux/mmc/core.h   |1 -
  include/linux/mmc/host.h   |   46 +---
  9 files changed, 25 insertions(+), 242 deletions(-)




-   if (host->caps&  MMC_CAP_DISABLE)
-   cancel_delayed_work(&host->disable);
cancel_delayed_work_sync(&host->detect);
mmc_flush_scheduled_work();

@@ -2402,13 +2245,11 @@ int mmc_suspend_host(struct mmc_host *host)
  {
int err = 0;

-   if (host->caps&  MMC_CAP_DISABLE)
-   cancel_delayed_work(&host->disable);
cancel_delayed_work(&host->detect);
mmc_flush_scheduled_work();
if (mmc_try_claim_host(host)) {
err = mmc_cache_ctrl(host, 0);
-   mmc_do_release_host(host);
+   mmc_release_host(host);


mmc_try_claim_host does not call host->ops->enable(), but 
mmc_release_host call host->ops->disable(), is there any reason for 
this? For example: If host controller disable method internally calls 
clk_disable() then that would cause reference count warnings.



} else {
err = -EBUSY;
}
@@ -2429,7 +2270,7 @@ int mmc_suspend_host(struct mmc_host *host)


Thanks,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: core: Ensure clocks are always enabled before host interaction

2012-01-23 Thread Sujit Reddy Thumma


Hi Linus Walleij,

On 12/30/2011 7:44 AM, Linus Walleij wrote:

On Mon, Dec 12, 2011 at 9:21 AM, Sujit Reddy Thumma
  wrote:


Ensure clocks are always enabled before any interaction with the
host controller driver. This makes sure that there is no race
between host execution and the core layer turning off clocks
in different context with clock gating framework.

Signed-off-by: Sujit Reddy Thumma


I guess Per Förlin may not be available, but would have preferred to
have his view on this as well, since he knows the semantics of
pre/post-req.


I have checked the implementation for pre-req and post-req in mmc host 
drivers. There is no interaction to the controller or card registers in 
these functions, but in future if drivers appeal to configure their 
controller in these functions then we must have clocks enabled.


Per, if you are available can you comment on this?



However from my PoV it looks nice and clean, and you surely have
done some serious testing on things like SDIO so:
Acked-by: Linus Walleij


Thanks. Tested with SD3.0, eMMC4.4 and SDIO2.0 cards.


Thanks,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: core: Ensure clocks are always enabled before host interaction

2011-12-26 Thread Sujit Reddy Thumma


On 12/12/2011 2:42 PM, Subhash Jadavani wrote:



-Original Message-
From: linux-mmc-ow...@vger.kernel.org [mailto:linux-mmc-
ow...@vger.kernel.org] On Behalf Of Sujit Reddy Thumma
Sent: Monday, December 12, 2011 1:52 PM
To: linux-mmc@vger.kernel.org
Cc: Sujit Reddy Thumma; c...@laptop.org
Subject: [PATCH] mmc: core: Ensure clocks are always enabled before
host interaction

Ensure clocks are always enabled before any interaction with the
host controller driver. This makes sure that there is no race
between host execution and the core layer turning off clocks
in different context with clock gating framework.


Thanks Sujit.

We were seeing the race between mmc_host_clk_gate_delayed and
execute_tuning(). Basically when host driver is in their execute_tuning()
ops, mmc_host_clk_gate_delayed() may disable the clocks which may cause the
execute_tuning() to fail. Your patch fixes this issue.

Acked-by: Subhash Jadavani


Chris/Linus, do you have any comments on this patch? Can this be pushed 
to mmc-next?


Thanks,
Sujit




Signed-off-by: Sujit Reddy Thumma
---
  drivers/mmc/core/core.c |   19 ---
  drivers/mmc/core/host.h |   21 -
  drivers/mmc/core/sd.c   |   22 ++
  drivers/mmc/core/sdio.c |   12 ++--
  drivers/mmc/core/sdio_irq.c |   10 --
  include/linux/mmc/host.h|   19 +++
  6 files changed, 71 insertions(+), 32 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index a2aa860..03ad9fa 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -290,8 +290,11 @@ static void mmc_wait_for_req_done(struct mmc_host
*host,
  static void mmc_pre_req(struct mmc_host *host, struct mmc_request
*mrq,
 bool is_first_req)
  {
-   if (host->ops->pre_req)
+   if (host->ops->pre_req) {
+   mmc_host_clk_hold(host);
host->ops->pre_req(host, mrq, is_first_req);
+   mmc_host_clk_release(host);
+   }
  }

  /**
@@ -306,8 +309,11 @@ static void mmc_pre_req(struct mmc_host *host,
struct mmc_request *mrq,
  static void mmc_post_req(struct mmc_host *host, struct mmc_request
*mrq,
 int err)
  {
-   if (host->ops->post_req)
+   if (host->ops->post_req) {
+   mmc_host_clk_hold(host);
host->ops->post_req(host, mrq, err);
+   mmc_host_clk_release(host);
+   }
  }

  /**
@@ -620,7 +626,9 @@ int mmc_host_enable(struct mmc_host *host)
int err;

host->en_dis_recurs = 1;
+   mmc_host_clk_hold(host);
err = host->ops->enable(host);
+   mmc_host_clk_release(host);
host->en_dis_recurs = 0;

if (err) {
@@ -640,7 +648,9 @@ static int mmc_host_do_disable(struct mmc_host
*host, int lazy)
int err;

host->en_dis_recurs = 1;
+   mmc_host_clk_hold(host);
err = host->ops->disable(host, lazy);
+   mmc_host_clk_release(host);
host->en_dis_recurs = 0;

if (err<  0) {
@@ -1203,8 +1213,11 @@ int mmc_set_signal_voltage(struct mmc_host
*host, int signal_voltage, bool cmd11

host->ios.signal_voltage = signal_voltage;

-   if (host->ops->start_signal_voltage_switch)
+   if (host->ops->start_signal_voltage_switch) {
+   mmc_host_clk_hold(host);
err = host->ops->start_signal_voltage_switch(host,&host-

ios);

+   mmc_host_clk_release(host);
+   }

return err;
  }
diff --git a/drivers/mmc/core/host.h b/drivers/mmc/core/host.h
index fb8a5cd..08a7852 100644
--- a/drivers/mmc/core/host.h
+++ b/drivers/mmc/core/host.h
@@ -14,27 +14,6 @@

  int mmc_register_host_class(void);
  void mmc_unregister_host_class(void);
-
-#ifdef CONFIG_MMC_CLKGATE
-void mmc_host_clk_hold(struct mmc_host *host);
-void mmc_host_clk_release(struct mmc_host *host);
-unsigned int mmc_host_clk_rate(struct mmc_host *host);
-
-#else
-static inline void mmc_host_clk_hold(struct mmc_host *host)
-{
-}
-
-static inline void mmc_host_clk_release(struct mmc_host *host)
-{
-}
-
-static inline unsigned int mmc_host_clk_rate(struct mmc_host *host)
-{
-   return host->ios.clock;
-}
-#endif
-
  void mmc_host_deeper_disable(struct work_struct *work);

  #endif
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 6f27d35..b5212b8 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -451,9 +451,11 @@ static int sd_select_driver_type(struct mmc_card
*card, u8 *status)
 * information and let the hardware specific code
 * return what is possible given the options
 */
+   mmc_host_clk_hold(card->host);
drive_strength = card->host->ops->select_drive_strength(
card

Re: [PATCH] mmc: use usleep_range() in mmc_delay()

2011-12-26 Thread Sujit Reddy Thumma


Hi Antipov,

Sorry for the delayed response. Please find some comments below:

On 12/21/2011 6:35 PM, Dmitry Antipov wrote:

On 12/21/2011 03:25 PM, Sujit Reddy Thumma wrote:


I have posted similar patch some time back.
http://comments.gmane.org/gmane.linux.ports.arm.msm/2119.

Would you like to comment on that?


- I believe we should forget about jiffies, HZ and other similar obsolete
timekeeping stuff;

- I have no ideas where did you get 'most typical' 20 ms. MMC subsystem
uses
mmc_delay() with two compile-time fixed values 1 and 10 ms, with the only
exception of card-dependent sleep/awake timeout. I was unable to find a
table
with typical values, but it's rounded up to >= 1 ms anyway.


The main aim of my patch was to fix mmc_delay() to give accurate delay.
You might want to refer to Documentation/timers/timers-howto.txt 
(Section: SLEEPING FOR ~USECS OR SMALL MSECS) to know why usleep_range() 
must be used instead of msleep for delays less than 20ms (or more 
accurately two jiffies, since in HZ=100 systems this comes to 20ms).


Also, in the documentation it is suggested that for delays greater than 
10ms+ use msleep(). Although MMC subsystem doesn't use mmc_delay() for 
greater than 10ms today but I guess we should keep it for future purpose 
and just not have only usleep_range() as your patch did.




Dmitry


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: use usleep_range() in mmc_delay()

2011-12-21 Thread Sujit Reddy Thumma


On 12/21/2011 12:26 PM, Dmitry Antipov wrote:

 From f447d78db65c6675e69466e8ed08364ff065ac08 Mon Sep 17 00:00:00 2001
From: Dmitry Antipov 
Date: Wed, 21 Dec 2011 10:51:03 +0400
Subject: [PATCH] mmc: use usleep_range() in mmc_delay()

---
drivers/mmc/core/core.h | 8 ++--
1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 14664f1..a77851e 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -47,12 +47,8 @@ void mmc_power_off(struct mmc_host *host);

static inline void mmc_delay(unsigned int ms)
{
- if (ms < 1000 / HZ) {
- cond_resched();
- mdelay(ms);
- } else {
- msleep(ms);
- }
+ unsigned long us = ms * USEC_PER_MSEC;
+ usleep_range(us, us + 1000);


I have posted similar patch some time back. 
http://comments.gmane.org/gmane.linux.ports.arm.msm/2119.


Would you like to comment on that?

Thanks,
Sujit


}

void mmc_rescan(struct work_struct *work);


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V4] mmc: Kill block requests if card is removed

2011-12-21 Thread Sujit Reddy Thumma


On 12/8/2011 2:52 PM, Adrian Hunter wrote:

On 08/12/11 10:35, Sujit Reddy Thumma wrote:

Kill block requests when the host realizes that the card is
removed from the slot and is sure that subsequent requests
are bound to fail. Do this silently so that the block
layer doesn't output unnecessary error messages.

Signed-off-by: Sujit Reddy Thumma


Acked-by: Adrian Hunter



Thanks Adrian.

Chris can you pull this patch with Adrian's ack?

Thanks,
Sujit



---
Changes in v4:
- Extra check to make sure no further commands are sent
in error case when the card is removed.
Changes in v3:
- Dropped dependency on Per's patch and is now dependent
on "[PATCH V4] mmc: allow upper layers to determine immediately
if a card has been removed" by Adrian Hunter.
- Modified commit text slightly as Adrian has
implemented his own suggestion in a different patch.

Changes in v2:
- Changed the implementation with further comments from Adrian
- Set the card removed flag in bus notifier callbacks
- This patch is now dependent on patch from Per Forlin:
http://thread.gmane.org/gmane.linux.kernel.mmc/11128/focus=11211
---
  drivers/mmc/card/block.c |   17 -
  drivers/mmc/card/queue.c |5 +
  2 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 0c959c9..0cad48a 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -121,6 +121,7 @@ enum mmc_blk_status {
MMC_BLK_ABORT,
MMC_BLK_DATA_ERR,
MMC_BLK_ECC_ERR,
+   MMC_BLK_NOMEDIUM,
  };

  module_param(perdev_minors, int, 0444);
@@ -639,6 +640,7 @@ static int get_card_status(struct mmc_card *card, u32 
*status, int retries)
return err;
  }

+#define ERR_NOMEDIUM   3
  #define ERR_RETRY 2
  #define ERR_ABORT 1
  #define ERR_CONTINUE  0
@@ -706,6 +708,9 @@ static int mmc_blk_cmd_recovery(struct mmc_card *card, 
struct request *req,
u32 status, stop_status = 0;
int err, retry;

+   if (mmc_card_removed(card))
+   return ERR_NOMEDIUM;
+
/*
 * Try to get card status which indicates both the card state
 * and why there was no response.  If the first attempt fails,
@@ -722,8 +727,12 @@ static int mmc_blk_cmd_recovery(struct mmc_card *card, 
struct request *req,
}

/* We couldn't get a response from the card.  Give up. */
-   if (err)
+   if (err) {
+   /* Check if the card is removed */
+   if (mmc_detect_card_removed(card->host))
+   return ERR_NOMEDIUM;
return ERR_ABORT;
+   }

/* Flag ECC errors */
if ((status&  R1_CARD_ECC_FAILED) ||
@@ -996,6 +1005,8 @@ static int mmc_blk_err_check(struct mmc_card *card,
return MMC_BLK_RETRY;
case ERR_ABORT:
return MMC_BLK_ABORT;
+   case ERR_NOMEDIUM:
+   return MMC_BLK_NOMEDIUM;
case ERR_CONTINUE:
break;
}
@@ -1329,6 +1340,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, 
struct request *rqc)
if (!ret)
goto start_new_req;
break;
+   case MMC_BLK_NOMEDIUM:
+   goto cmd_abort;
}

if (ret) {
@@ -1345,6 +1358,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, 
struct request *rqc)

   cmd_abort:
spin_lock_irq(&md->lock);
+   if (mmc_card_removed(card))
+   req->cmd_flags |= REQ_QUIET;
while (ret)
ret = __blk_end_request(req, -EIO, blk_rq_cur_bytes(req));
spin_unlock_irq(&md->lock);
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index dcad59c..2517547 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -29,6 +29,8 @@
   */
  static int mmc_prep_request(struct request_queue *q, struct request *req)
  {
+   struct mmc_queue *mq = q->queuedata;
+
/*
 * We only like normal block requests and discards.
 */
@@ -37,6 +39,9 @@ static int mmc_prep_request(struct request_queue *q, struct 
request *req)
return BLKPREP_KILL;
}

+   if (mq&&  mmc_card_removed(mq->card))
+   return BLKPREP_KILL;
+
req->cmd_flags |= REQ_DONTPREP;

return BLKPREP_OK;


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] mmc: core: Ensure clocks are always enabled before host interaction

2011-12-12 Thread Sujit Reddy Thumma

Ensure clocks are always enabled before any interaction with the
host controller driver. This makes sure that there is no race
between host execution and the core layer turning off clocks
in different context with clock gating framework.

Signed-off-by: Sujit Reddy Thumma 
---
 drivers/mmc/core/core.c |   19 ---
 drivers/mmc/core/host.h |   21 -
 drivers/mmc/core/sd.c   |   22 ++
 drivers/mmc/core/sdio.c |   12 ++--
 drivers/mmc/core/sdio_irq.c |   10 --
 include/linux/mmc/host.h|   19 +++
 6 files changed, 71 insertions(+), 32 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index a2aa860..03ad9fa 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -290,8 +290,11 @@ static void mmc_wait_for_req_done(struct mmc_host *host,
 static void mmc_pre_req(struct mmc_host *host, struct mmc_request *mrq,
 bool is_first_req)
 {
-   if (host->ops->pre_req)
+   if (host->ops->pre_req) {
+   mmc_host_clk_hold(host);
host->ops->pre_req(host, mrq, is_first_req);
+   mmc_host_clk_release(host);
+   }
 }
 
 /**
@@ -306,8 +309,11 @@ static void mmc_pre_req(struct mmc_host *host, struct 
mmc_request *mrq,
 static void mmc_post_req(struct mmc_host *host, struct mmc_request *mrq,
 int err)
 {
-   if (host->ops->post_req)
+   if (host->ops->post_req) {
+   mmc_host_clk_hold(host);
host->ops->post_req(host, mrq, err);
+   mmc_host_clk_release(host);
+   }
 }
 
 /**
@@ -620,7 +626,9 @@ int mmc_host_enable(struct mmc_host *host)
int err;
 
host->en_dis_recurs = 1;
+   mmc_host_clk_hold(host);
err = host->ops->enable(host);
+   mmc_host_clk_release(host);
host->en_dis_recurs = 0;
 
if (err) {
@@ -640,7 +648,9 @@ static int mmc_host_do_disable(struct mmc_host *host, int 
lazy)
int err;
 
host->en_dis_recurs = 1;
+   mmc_host_clk_hold(host);
err = host->ops->disable(host, lazy);
+   mmc_host_clk_release(host);
host->en_dis_recurs = 0;
 
if (err < 0) {
@@ -1203,8 +1213,11 @@ int mmc_set_signal_voltage(struct mmc_host *host, int 
signal_voltage, bool cmd11
 
host->ios.signal_voltage = signal_voltage;
 
-   if (host->ops->start_signal_voltage_switch)
+   if (host->ops->start_signal_voltage_switch) {
+   mmc_host_clk_hold(host);
err = host->ops->start_signal_voltage_switch(host, &host->ios);
+   mmc_host_clk_release(host);
+   }
 
return err;
 }
diff --git a/drivers/mmc/core/host.h b/drivers/mmc/core/host.h
index fb8a5cd..08a7852 100644
--- a/drivers/mmc/core/host.h
+++ b/drivers/mmc/core/host.h
@@ -14,27 +14,6 @@
 
 int mmc_register_host_class(void);
 void mmc_unregister_host_class(void);
-
-#ifdef CONFIG_MMC_CLKGATE
-void mmc_host_clk_hold(struct mmc_host *host);
-void mmc_host_clk_release(struct mmc_host *host);
-unsigned int mmc_host_clk_rate(struct mmc_host *host);
-
-#else
-static inline void mmc_host_clk_hold(struct mmc_host *host)
-{
-}
-
-static inline void mmc_host_clk_release(struct mmc_host *host)
-{
-}
-
-static inline unsigned int mmc_host_clk_rate(struct mmc_host *host)
-{
-   return host->ios.clock;
-}
-#endif
-
 void mmc_host_deeper_disable(struct work_struct *work);
 
 #endif
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 6f27d35..b5212b8 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -451,9 +451,11 @@ static int sd_select_driver_type(struct mmc_card *card, u8 
*status)
 * information and let the hardware specific code
 * return what is possible given the options
 */
+   mmc_host_clk_hold(card->host);
drive_strength = card->host->ops->select_drive_strength(
card->sw_caps.uhs_max_dtr,
host_drv_type, card_drv_type);
+   mmc_host_clk_release(card->host);
 
err = mmc_sd_switch(card, 1, 2, drive_strength, status);
if (err)
@@ -660,8 +662,11 @@ static int mmc_sd_init_uhs_card(struct mmc_card *card)
goto out;
 
/* SPI mode doesn't define CMD19 */
-   if (!mmc_host_is_spi(card->host) && card->host->ops->execute_tuning)
+   if (!mmc_host_is_spi(card->host) && card->host->ops->execute_tuning) {
+   mmc_host_clk_hold(card->host);
err = card->host->ops->execute_tuning(card->host);
+   mmc_host_clk_release(card->host);
+   }
 
 out:
kfree(status);
@@ -849,8 +854,11 @@ int mmc_sd_setup

[PATCH V4] mmc: Kill block requests if card is removed

2011-12-08 Thread Sujit Reddy Thumma

Kill block requests when the host realizes that the card is
removed from the slot and is sure that subsequent requests
are bound to fail. Do this silently so that the block
layer doesn't output unnecessary error messages.

Signed-off-by: Sujit Reddy Thumma 

---
Changes in v4:
- Extra check to make sure no further commands are sent
in error case when the card is removed.
Changes in v3:
- Dropped dependency on Per's patch and is now dependent
on "[PATCH V4] mmc: allow upper layers to determine immediately
if a card has been removed" by Adrian Hunter.
- Modified commit text slightly as Adrian has
implemented his own suggestion in a different patch.

Changes in v2:
- Changed the implementation with further comments from Adrian
- Set the card removed flag in bus notifier callbacks
- This patch is now dependent on patch from Per Forlin:
http://thread.gmane.org/gmane.linux.kernel.mmc/11128/focus=11211
---
 drivers/mmc/card/block.c |   17 -
 drivers/mmc/card/queue.c |5 +
 2 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 0c959c9..0cad48a 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -121,6 +121,7 @@ enum mmc_blk_status {
MMC_BLK_ABORT,
MMC_BLK_DATA_ERR,
MMC_BLK_ECC_ERR,
+   MMC_BLK_NOMEDIUM,
 };
 
 module_param(perdev_minors, int, 0444);
@@ -639,6 +640,7 @@ static int get_card_status(struct mmc_card *card, u32 
*status, int retries)
return err;
 }
 
+#define ERR_NOMEDIUM   3
 #define ERR_RETRY  2
 #define ERR_ABORT  1
 #define ERR_CONTINUE   0
@@ -706,6 +708,9 @@ static int mmc_blk_cmd_recovery(struct mmc_card *card, 
struct request *req,
u32 status, stop_status = 0;
int err, retry;
 
+   if (mmc_card_removed(card))
+   return ERR_NOMEDIUM;
+
/*
 * Try to get card status which indicates both the card state
 * and why there was no response.  If the first attempt fails,
@@ -722,8 +727,12 @@ static int mmc_blk_cmd_recovery(struct mmc_card *card, 
struct request *req,
}
 
/* We couldn't get a response from the card.  Give up. */
-   if (err)
+   if (err) {
+   /* Check if the card is removed */
+   if (mmc_detect_card_removed(card->host))
+   return ERR_NOMEDIUM;
return ERR_ABORT;
+   }
 
/* Flag ECC errors */
if ((status & R1_CARD_ECC_FAILED) ||
@@ -996,6 +1005,8 @@ static int mmc_blk_err_check(struct mmc_card *card,
return MMC_BLK_RETRY;
case ERR_ABORT:
return MMC_BLK_ABORT;
+   case ERR_NOMEDIUM:
+   return MMC_BLK_NOMEDIUM;
case ERR_CONTINUE:
break;
}
@@ -1329,6 +1340,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, 
struct request *rqc)
if (!ret)
goto start_new_req;
break;
+   case MMC_BLK_NOMEDIUM:
+   goto cmd_abort;
}
 
if (ret) {
@@ -1345,6 +1358,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, 
struct request *rqc)
 
  cmd_abort:
spin_lock_irq(&md->lock);
+   if (mmc_card_removed(card))
+   req->cmd_flags |= REQ_QUIET;
while (ret)
ret = __blk_end_request(req, -EIO, blk_rq_cur_bytes(req));
spin_unlock_irq(&md->lock);
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index dcad59c..2517547 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -29,6 +29,8 @@
  */
 static int mmc_prep_request(struct request_queue *q, struct request *req)
 {
+   struct mmc_queue *mq = q->queuedata;
+
/*
 * We only like normal block requests and discards.
 */
@@ -37,6 +39,9 @@ static int mmc_prep_request(struct request_queue *q, struct 
request *req)
return BLKPREP_KILL;
}
 
+   if (mq && mmc_card_removed(mq->card))
+   return BLKPREP_KILL;
+
req->cmd_flags |= REQ_DONTPREP;
 
return BLKPREP_OK;
-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3] mmc: Kill block requests if card is removed

2011-12-06 Thread Sujit Reddy Thumma


Hi Adrian/Per,

Any comments on this patch?

Thanks,
Sujit

On 12/1/2011 4:00 PM, Sujit Reddy Thumma wrote:

Kill block requests when the host realizes that the card is
removed from the slot and is sure that subsequent requests
are bound to fail. Do this silently so that the block
layer doesn't output unnecessary error messages.

Signed-off-by: Sujit Reddy Thumma

---
Changes in v3:
- Dropped dependency on Per's patch and is now dependent
on "[PATCH V4] mmc: allow upper layers to determine immediately
if a card has been removed" by Adrian Hunter.
- Modified commit text slightly as Adrian has
implemented his own suggestion in a different patch.

Changes in v2:
- Changed the implementation with further comments from Adrian
- Set the card removed flag in bus notifier callbacks
- This patch is now dependent on patch from Per Forlin:
http://thread.gmane.org/gmane.linux.kernel.mmc/11128/focus=11211
---
  drivers/mmc/card/block.c |   14 +-
  drivers/mmc/card/queue.c |5 +
  2 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index c80bb6d..20350cb 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -119,6 +119,7 @@ enum mmc_blk_status {
MMC_BLK_ABORT,
MMC_BLK_DATA_ERR,
MMC_BLK_ECC_ERR,
+   MMC_BLK_NOMEDIUM,
  };

  module_param(perdev_minors, int, 0444);
@@ -565,6 +566,7 @@ static int get_card_status(struct mmc_card *card, u32 
*status, int retries)
return err;
  }

+#define ERR_NOMEDIUM   3
  #define ERR_RETRY 2
  #define ERR_ABORT 1
  #define ERR_CONTINUE  0
@@ -648,8 +650,12 @@ static int mmc_blk_cmd_recovery(struct mmc_card *card, 
struct request *req,
}

/* We couldn't get a response from the card.  Give up. */
-   if (err)
+   if (err) {
+   /* Check if the card is removed */
+   if (mmc_detect_card_removed(card->host))
+   return ERR_NOMEDIUM;
return ERR_ABORT;
+   }

/* Flag ECC errors */
if ((status&  R1_CARD_ECC_FAILED) ||
@@ -922,6 +928,8 @@ static int mmc_blk_err_check(struct mmc_card *card,
return MMC_BLK_RETRY;
case ERR_ABORT:
return MMC_BLK_ABORT;
+   case ERR_NOMEDIUM:
+   return MMC_BLK_NOMEDIUM;
case ERR_CONTINUE:
break;
}
@@ -1255,6 +1263,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, 
struct request *rqc)
if (!ret)
goto start_new_req;
break;
+   case MMC_BLK_NOMEDIUM:
+   goto cmd_abort;
}

if (ret) {
@@ -1271,6 +1281,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, 
struct request *rqc)

   cmd_abort:
spin_lock_irq(&md->lock);
+   if (mmc_card_removed(card))
+   req->cmd_flags |= REQ_QUIET;
while (ret)
ret = __blk_end_request(req, -EIO, blk_rq_cur_bytes(req));
spin_unlock_irq(&md->lock);
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index dcad59c..2517547 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -29,6 +29,8 @@
   */
  static int mmc_prep_request(struct request_queue *q, struct request *req)
  {
+   struct mmc_queue *mq = q->queuedata;
+
/*
 * We only like normal block requests and discards.
 */
@@ -37,6 +39,9 @@ static int mmc_prep_request(struct request_queue *q, struct 
request *req)
return BLKPREP_KILL;
}

+   if (mq&&  mmc_card_removed(mq->card))
+   return BLKPREP_KILL;
+
req->cmd_flags |= REQ_DONTPREP;

return BLKPREP_OK;


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH-next] mmc: core: Fixup delayed work clock gating patch

2011-12-06 Thread Sujit Reddy Thumma


On 12/5/2011 11:58 PM, Stephen Boyd wrote:

c31b50e (mmc: core: Use delayed work in clock gating framework,
2011-11-14) missed a few things during review:

  o A useless pr_info()

  o milliseconds was written as two words

  o The sysfs file had units in its output

Fix all three problems.

Cc: Sujit Reddy Thumma
Signed-off-by: Stephen Boyd
---

Feel free to squash if desired.

  Documentation/mmc/mmc-dev-attrs.txt |2 +-
  drivers/mmc/core/host.c |6 +-
  2 files changed, 2 insertions(+), 6 deletions(-)


Thanks Stephen.



diff --git a/Documentation/mmc/mmc-dev-attrs.txt 
b/Documentation/mmc/mmc-dev-attrs.txt
index b024556..22ae844 100644
--- a/Documentation/mmc/mmc-dev-attrs.txt
+++ b/Documentation/mmc/mmc-dev-attrs.txt
@@ -71,6 +71,6 @@ SD/MMC/SDIO Clock Gating Attribute
  Read and write access is provided to following attribute.
  This attribute appears only if CONFIG_MMC_CLKGATE is enabled.

-   clkgate_delay   Tune the clock gating delay with desired value in milli 
seconds.
+   clkgate_delay   Tune the clock gating delay with desired value in 
milliseconds.

  echo  >  /sys/class/mmc_host/mmcX/clkgate_delay
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 835e86a..c152ce0c 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -58,8 +58,7 @@ static ssize_t clkgate_delay_show(struct device *dev,
struct device_attribute *attr, char *buf)
  {
struct mmc_host *host = cls_dev_to_mmc_host(dev);
-   return snprintf(buf, PAGE_SIZE, "%lu millisecs\n",
-   host->clkgate_delay);
+   return snprintf(buf, PAGE_SIZE, "%lu\n", host->clkgate_delay);
  }

  static ssize_t clkgate_delay_store(struct device *dev,
@@ -74,9 +73,6 @@ static ssize_t clkgate_delay_store(struct device *dev,
spin_lock_irqsave(&host->clk_lock, flags);
host->clkgate_delay = value;
spin_unlock_irqrestore(&host->clk_lock, flags);
-
-   pr_info("%s: clock gate delay set to %lu ms\n",
-   mmc_hostname(host), value);
return count;
  }


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: mmc_test: align max_seg_size

2011-12-02 Thread Sujit Reddy Thumma


> Hi,
>
> On Mon, Nov 14 2011, Per Forlin wrote:
>> If max_seg_size is unaligned, mmc_test_map_sg() may create sg element
>> sizes that are not aligned with 512 byte. Fix, align max_seg_size at
>> mmc_test_area_init().
>>
>> Signed-off-by: Per Forlin 
>> ---
>>  drivers/mmc/card/mmc_test.c |1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
>> index b038c4a..5848998 100644
>> --- a/drivers/mmc/card/mmc_test.c
>> +++ b/drivers/mmc/card/mmc_test.c
>> @@ -1581,6 +1581,7 @@ static int mmc_test_area_init(struct mmc_test_card
>> *test, int erase, int fill)
>>
>>  t->max_segs = test->card->host->max_segs;
>>  t->max_seg_sz = test->card->host->max_seg_size;
>> +t->max_seg_sz -= t->max_seg_sz % 512;
>>
>>  t->max_tfr = t->max_sz;
>>  if (t->max_tfr >> 9 > test->card->host->max_blk_count)
>
> Thanks, pushed to mmc-next for 3.2.  (Sujit, please let us know if you
> still have worries about aligning to host->max_blk_size.)

No issues. The change looks good.

-- 
Thanks,
Sujit

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V4] mmc: allow upper layers to determine immediately if a card has been removed

2011-12-01 Thread Sujit Reddy Thumma


On 11/28/2011 7:52 PM, Adrian Hunter wrote:

Add a function mmc_detect_card_removed() which upper layers
can use to determine immediately if a card has been removed.
This function should be called after an I/O request fails so
that all queued I/O requests can be errored out immediately
instead of waiting for the card device to be removed.

Signed-off-by: Adrian Hunter

Acked-by: Sujit Reddy Thumma 

--
Thanks,
Sujit

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V3] mmc: Kill block requests if card is removed

2011-12-01 Thread Sujit Reddy Thumma

Kill block requests when the host realizes that the card is
removed from the slot and is sure that subsequent requests
are bound to fail. Do this silently so that the block
layer doesn't output unnecessary error messages.

Signed-off-by: Sujit Reddy Thumma 

---
Changes in v3:
- Dropped dependency on Per's patch and is now dependent
on "[PATCH V4] mmc: allow upper layers to determine immediately
if a card has been removed" by Adrian Hunter.
- Modified commit text slightly as Adrian has
implemented his own suggestion in a different patch.

Changes in v2:
- Changed the implementation with further comments from Adrian
- Set the card removed flag in bus notifier callbacks
- This patch is now dependent on patch from Per Forlin:
http://thread.gmane.org/gmane.linux.kernel.mmc/11128/focus=11211
---
 drivers/mmc/card/block.c |   14 +-
 drivers/mmc/card/queue.c |5 +
 2 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index c80bb6d..20350cb 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -119,6 +119,7 @@ enum mmc_blk_status {
MMC_BLK_ABORT,
MMC_BLK_DATA_ERR,
MMC_BLK_ECC_ERR,
+   MMC_BLK_NOMEDIUM,
 };
 
 module_param(perdev_minors, int, 0444);
@@ -565,6 +566,7 @@ static int get_card_status(struct mmc_card *card, u32 
*status, int retries)
return err;
 }
 
+#define ERR_NOMEDIUM   3
 #define ERR_RETRY  2
 #define ERR_ABORT  1
 #define ERR_CONTINUE   0
@@ -648,8 +650,12 @@ static int mmc_blk_cmd_recovery(struct mmc_card *card, 
struct request *req,
}
 
/* We couldn't get a response from the card.  Give up. */
-   if (err)
+   if (err) {
+   /* Check if the card is removed */
+   if (mmc_detect_card_removed(card->host))
+   return ERR_NOMEDIUM;
return ERR_ABORT;
+   }
 
/* Flag ECC errors */
if ((status & R1_CARD_ECC_FAILED) ||
@@ -922,6 +928,8 @@ static int mmc_blk_err_check(struct mmc_card *card,
return MMC_BLK_RETRY;
case ERR_ABORT:
return MMC_BLK_ABORT;
+   case ERR_NOMEDIUM:
+   return MMC_BLK_NOMEDIUM;
case ERR_CONTINUE:
break;
}
@@ -1255,6 +1263,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, 
struct request *rqc)
if (!ret)
goto start_new_req;
break;
+   case MMC_BLK_NOMEDIUM:
+   goto cmd_abort;
}
 
if (ret) {
@@ -1271,6 +1281,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, 
struct request *rqc)
 
  cmd_abort:
spin_lock_irq(&md->lock);
+   if (mmc_card_removed(card))
+   req->cmd_flags |= REQ_QUIET;
while (ret)
ret = __blk_end_request(req, -EIO, blk_rq_cur_bytes(req));
spin_unlock_irq(&md->lock);
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index dcad59c..2517547 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -29,6 +29,8 @@
  */
 static int mmc_prep_request(struct request_queue *q, struct request *req)
 {
+   struct mmc_queue *mq = q->queuedata;
+
/*
 * We only like normal block requests and discards.
 */
@@ -37,6 +39,9 @@ static int mmc_prep_request(struct request_queue *q, struct 
request *req)
return BLKPREP_KILL;
}
 
+   if (mq && mmc_card_removed(mq->card))
+   return BLKPREP_KILL;
+
req->cmd_flags |= REQ_DONTPREP;
 
return BLKPREP_OK;
-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3] mmc: allow upper layers to determine immediately if a card has been removed

2011-11-28 Thread Sujit Reddy Thumma


> Add a function mmc_detect_card_removed() which upper layers
> can use to determine immediately if a card has been removed.
> This function should be called after an I/O request fails so
> that all queued I/O requests can be errored out immediately
> instead of waiting for the card device to be removed.
>
> Signed-off-by: Adrian Hunter 
> ---
>  drivers/mmc/core/core.c  |   51
> +++--
>  drivers/mmc/core/core.h  |3 ++
>  drivers/mmc/core/mmc.c   |   12 ++-
>  drivers/mmc/core/sd.c|   12 ++-
>  drivers/mmc/core/sdio.c  |   11 +-
>  include/linux/mmc/card.h |3 ++
>  include/linux/mmc/core.h |2 +
>  include/linux/mmc/host.h |1 +
>  8 files changed, 89 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 271efea..3dacc98 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -140,7 +140,7 @@ void mmc_request_done(struct mmc_host *host, struct
> mmc_request *mrq)
>   cmd->retries = 0;
>   }
>
> - if (err && cmd->retries) {
> + if (err && cmd->retries && !mmc_card_removed(host->card)) {
>   /*
>* Request starter must handle retries - see
>* mmc_wait_for_req_done().
> @@ -247,6 +247,11 @@ static void __mmc_start_req(struct mmc_host *host,
> struct mmc_request *mrq)
>  {
>   init_completion(&mrq->completion);
>   mrq->done = mmc_wait_done;
> + if (mmc_card_removed(host->card)) {
> + mrq->cmd->error = -ENOMEDIUM;
> + complete(&mrq->completion);
> + return;
> + }
>   mmc_start_request(host, mrq);
>  }
>
> @@ -259,7 +264,8 @@ static void mmc_wait_for_req_done(struct mmc_host
> *host,
>   wait_for_completion(&mrq->completion);
>
>   cmd = mrq->cmd;
> - if (!cmd->error || !cmd->retries)
> + if (!cmd->error || !cmd->retries ||
> + mmc_card_removed(host->card))
>   break;
>
>   pr_debug("%s: req failed (CMD%u): %d, retrying...\n",
> @@ -1456,7 +1462,7 @@ void mmc_detect_change(struct mmc_host *host,
> unsigned long delay)
>   WARN_ON(host->removed);
>   spin_unlock_irqrestore(&host->lock, flags);
>  #endif
> -
> + host->detect_change = 1;
>   mmc_schedule_delayed_work(&host->detect, delay);
>  }
>
> @@ -2049,6 +2055,43 @@ static int mmc_rescan_try_freq(struct mmc_host
> *host, unsigned freq)
>   return -EIO;
>  }
>
> +int _mmc_detect_card_removed(struct mmc_host *host)
> +{
> + int ret;
> +
> + if (!(host->caps & MMC_CAP_NONREMOVABLE) || !host->bus_ops->alive)
> + return 0;
> +
> + if (!host->card || mmc_card_removed(host->card))
> + return 1;
> +
> + ret = host->bus_ops->alive(host);
> + if (ret) {
> + mmc_card_set_removed(host->card);
> + pr_info("%s: card removed\n", mmc_hostname(host));

We are printing the same information when mmc_remove_card() is called. Can
we move this to pr_debug() here?


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3] mmc: allow upper layers to determine immediately if a card has been removed

2011-11-28 Thread Sujit Reddy Thumma


> Add a function mmc_detect_card_removed() which upper layers
> can use to determine immediately if a card has been removed.
> This function should be called after an I/O request fails so
> that all queued I/O requests can be errored out immediately
> instead of waiting for the card device to be removed.
>
> Signed-off-by: Adrian Hunter 
> ---
>  drivers/mmc/core/core.c  |   51
> +++--
>  drivers/mmc/core/core.h  |3 ++
>  drivers/mmc/core/mmc.c   |   12 ++-
>  drivers/mmc/core/sd.c|   12 ++-
>  drivers/mmc/core/sdio.c  |   11 +-
>  include/linux/mmc/card.h |3 ++
>  include/linux/mmc/core.h |2 +
>  include/linux/mmc/host.h |1 +
>  8 files changed, 89 insertions(+), 6 deletions(-)
>
...

> +int _mmc_detect_card_removed(struct mmc_host *host)
> +{
> + int ret;
> +
> + if (!(host->caps & MMC_CAP_NONREMOVABLE) || !host->bus_ops->alive)

This should be just "if ((host->caps & MMC_CAP_NONREMOVABLE) || ...)"

Otherwise, Acked-by: Sujit Reddy Thumma 

> + return 0;
> +
> + if (!host->card || mmc_card_removed(host->card))
> + return 1;
> +
> + ret = host->bus_ops->alive(host);
> + if (ret) {
> + mmc_card_set_removed(host->card);
> + pr_info("%s: card removed\n", mmc_hostname(host));
> + }
> +
> + return ret;
> +}
> +


-- 
Thanks
Sujit

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2] mmc: Kill block requests if card is removed

2011-11-27 Thread Sujit Reddy Thumma


Hi Adrian,

On 11/25/2011 4:58 PM, Adrian Hunter wrote:

On 25/11/11 12:26, Sujit Reddy Thumma wrote:

Hi Adrian,

On 11/24/2011 8:22 PM, Adrian Hunter wrote:

Hi

Here is a way to allow mmc block to determine immediately
if a card has been removed while preserving the present rules
and keeping card detection in the bus_ops.

Untested I'm afraid.

Regards
Adrian



  From 2c6c9535b94c07fa3d12af26e9b6de500cb29970 Mon Sep 17 00:00:00 2001
From: Adrian Hunter
Date: Thu, 24 Nov 2011 16:32:34 +0200
Subject: [PATCH] mmc: allow upper layers to determine immediately if a
card has been removed

Add a function mmc_card_removed() which upper layers can use
to determine immediately if a card has been removed.  This
function should be called after an I/O request fails so that
all queued I/O requests can be errored out immediately
instead of waiting for the card device to be removed.

Signed-off-by: Adrian Hunter
---
   drivers/mmc/core/core.c  |   32 
   drivers/mmc/core/core.h  |3 +++
   drivers/mmc/core/mmc.c   |   12 +++-
   drivers/mmc/core/sd.c|   12 +++-
   drivers/mmc/core/sdio.c  |   11 ++-
   include/linux/mmc/card.h |1 +
   include/linux/mmc/core.h |2 ++
   7 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 271efea..c317564 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2049,6 +2049,38 @@ static int mmc_rescan_try_freq(struct mmc_host
*host, unsigned freq)
   return -EIO;
   }

+int _mmc_card_removed(struct mmc_host *host, int detect_change)
+{
+int ret;
+
+if (!(host->caps&   MMC_CAP_NONREMOVABLE) || !host->bus_ops->alive)
+return 0;
+
+if (!host->card || (host->card->state&   MMC_CARD_REMOVED))
+return 1;
+
+/*
+ * The card will be considered alive unless we have been asked to detect
+ * a change or host requires polling to provide card detection.
+ */
+if (!detect_change&&   !(host->caps&   MMC_CAP_NEEDS_POLL))
+return 0;
+
+ret = host->bus_ops->alive(host);
+if (ret)
+host->card->state |= MMC_CARD_REMOVED;
+
+return ret;
+}
+


The patch itself looks good to me, but can't we decide this in the block
layer itself when we issue get_card_status() when the ongoing request fails?

--
 for (retry = 2; retry>= 0; retry--) {
 err = get_card_status(card,&status, 0);
 if (!err)
 break;

 prev_cmd_status_valid = false;
 pr_err("%s: error %d sending status command, %sing\n",
req->rq_disk->disk_name, err, retry ? "retry" :
"abort");
 }


  /* We couldn't get a response from the card.  Give up. */
-if (err)
+if (err) {
+/*
+ * If the card didn't respond to status command,
+ * it is likely that card is gone. Flag it as removed,
+ * mmc_detect_change() cleans the rest.
+ */
+mmc_card_set_removed(card);
  return ERR_ABORT;
+}


The V2 patch I have posted takes care of this. Please let me know if it not
good to decide in the block layer itself. Essentially, both the
implementations are sending CMD13 to know the status of the card.


I think it is better to have the decision over whether or not the card
has been removed in only one place.  Also, never flagging
MMC_CAP_NONREMOVABLE cards as removed nor if the HC driver has not called
mmc_detect_change.


Agreed. This is much cleaner implementation. Thanks.



But the block change is essentially the same:

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index c80bb6d..32590c3 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -632,6 +632,8 @@ static int mmc_blk_cmd_recovery(struct mmc_card *card,
struct request *req,
u32 status, stop_status = 0;
int err, retry;

+   if (mmc_card_removed(card))
+   return ERR_ABORT;
/*
 * Try to get card status which indicates both the card state
 * and why there was no response.  If the first attempt fails,
@@ -648,8 +650,10 @@ static int mmc_blk_cmd_recovery(struct mmc_card *card,
struct request *req,
}

/* We couldn't get a response from the card.  Give up. */
-   if (err)
+   if (err) {
+   mmc_detect_card_removed(card->host);
return ERR_ABORT;
+   }

/* Flag ECC errors */
if ((status&  R1_CARD_ECC_FAILED) ||

I attached a revised version of the patch adding -ENOMEDIUM
errors from __mmc_start_request as you and Per discussed.


Thanks. I had a look at it and I have some comments:

+int mmc_detect_card_removed(struct mmc_

Re: [PATCH V2] mmc: Kill block requests if card is removed

2011-11-25 Thread Sujit Reddy Thumma


Hi Adrian,

On 11/24/2011 8:22 PM, Adrian Hunter wrote:

Hi

Here is a way to allow mmc block to determine immediately
if a card has been removed while preserving the present rules
and keeping card detection in the bus_ops.

Untested I'm afraid.

Regards
Adrian



 From 2c6c9535b94c07fa3d12af26e9b6de500cb29970 Mon Sep 17 00:00:00 2001
From: Adrian Hunter
Date: Thu, 24 Nov 2011 16:32:34 +0200
Subject: [PATCH] mmc: allow upper layers to determine immediately if a card has 
been removed

Add a function mmc_card_removed() which upper layers can use
to determine immediately if a card has been removed.  This
function should be called after an I/O request fails so that
all queued I/O requests can be errored out immediately
instead of waiting for the card device to be removed.

Signed-off-by: Adrian Hunter
---
  drivers/mmc/core/core.c  |   32 
  drivers/mmc/core/core.h  |3 +++
  drivers/mmc/core/mmc.c   |   12 +++-
  drivers/mmc/core/sd.c|   12 +++-
  drivers/mmc/core/sdio.c  |   11 ++-
  include/linux/mmc/card.h |1 +
  include/linux/mmc/core.h |2 ++
  7 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 271efea..c317564 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2049,6 +2049,38 @@ static int mmc_rescan_try_freq(struct mmc_host *host, 
unsigned freq)
return -EIO;
  }

+int _mmc_card_removed(struct mmc_host *host, int detect_change)
+{
+   int ret;
+
+   if (!(host->caps&  MMC_CAP_NONREMOVABLE) || !host->bus_ops->alive)
+   return 0;
+
+   if (!host->card || (host->card->state&  MMC_CARD_REMOVED))
+   return 1;
+
+   /*
+* The card will be considered alive unless we have been asked to detect
+* a change or host requires polling to provide card detection.
+*/
+   if (!detect_change&&  !(host->caps&  MMC_CAP_NEEDS_POLL))
+   return 0;
+
+   ret = host->bus_ops->alive(host);
+   if (ret)
+   host->card->state |= MMC_CARD_REMOVED;
+
+   return ret;
+}
+


The patch itself looks good to me, but can't we decide this in the block 
layer itself when we issue get_card_status() when the ongoing request fails?


--
for (retry = 2; retry >= 0; retry--) {
err = get_card_status(card, &status, 0);
if (!err)
break;

prev_cmd_status_valid = false;
pr_err("%s: error %d sending status command, %sing\n",
   req->rq_disk->disk_name, err, retry ? "retry" : 
"abort");

}


/* We couldn't get a response from the card.  Give up. */
-   if (err)
+   if (err) {
+   /*
+* If the card didn't respond to status command,
+* it is likely that card is gone. Flag it as removed,
+* mmc_detect_change() cleans the rest.
+*/
+   mmc_card_set_removed(card);
return ERR_ABORT;
+   }


The V2 patch I have posted takes care of this. Please let me know if it 
not good to decide in the block layer itself. Essentially, both the 
implementations are sending CMD13 to know the status of the card.



+int mmc_card_removed(struct mmc_host *host)
+{
+   WARN_ON(!host->claimed);
+
+   return _mmc_card_removed(host, work_pending(&host->detect.work));
+}
+EXPORT_SYMBOL(mmc_card_removed);
+
  void mmc_rescan(struct work_struct *work)
  {
static const unsigned freqs[] = { 40, 30, 20, 10 };
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 14664f1..4b6b10e 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -24,6 +24,7 @@ struct mmc_bus_ops {
int (*resume)(struct mmc_host *);
int (*power_save)(struct mmc_host *);
int (*power_restore)(struct mmc_host *);
+   int (*alive)(struct mmc_host *);
  };

  void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops);
@@ -59,6 +60,8 @@ void mmc_rescan(struct work_struct *work);
  void mmc_start_host(struct mmc_host *host);
  void mmc_stop_host(struct mmc_host *host);

+int _mmc_card_removed(struct mmc_host *host, int detect_change);
+
  int mmc_attach_mmc(struct mmc_host *host);
  int mmc_attach_sd(struct mmc_host *host);
  int mmc_attach_sdio(struct mmc_host *host);
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index a1223bd..5841d08 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1104,6 +1104,14 @@ static void mmc_remove(struct mmc_host *host)
  }

  /*
+ * Card detection - card is alive.
+ */
+static int mmc_alive(struct mmc_host *host)
+{
+   return mmc_send_status(host->card, NULL);
+}
+
+/*
   * Card detection callback from host.
   */
  static void mmc_detect(struct mmc

Re: [PATCH V2] mmc: Kill block requests if card is removed

2011-11-24 Thread Sujit Reddy Thumma


> On Thu, Nov 24, 2011 at 12:27 PM, Sujit Reddy Thumma
>  wrote:
>> Hi Per,
>>
>> On 11/22/2011 6:40 PM, Per Forlin wrote:
>>>
>>> Hi Sujit,
>>>
>>> On Tue, Nov 22, 2011 at 11:56 AM, Sujit Reddy Thumma
>>>   wrote:
>>>>
>>>> Kill block requests when the host knows that the card is
>>>> removed from the slot and is sure that subsequent requests
>>>> are bound to fail. Do this silently so that the block
>>>> layer doesn't output unnecessary error messages.
>>>>
>>>> This patch implements suggestion from Adrian Hunter,
>>>> http://thread.gmane.org/gmane.linux.kernel.mmc/2714/focus=3474
>>>>
>>>> Signed-off-by: Sujit Reddy Thumma
>>>>
>>>> ---
>>>> Changes in v2:
>>>>        - Changed the implementation with further comments from Adrian
>>>>        - Set the card removed flag in bus notifier callbacks
>>>>        - This patch is now dependent on patch from Per Forlin:
>>>>      
>>>>  http://thread.gmane.org/gmane.linux.kernel.mmc/11128/focus=11211
>>>> ---
>>>>  drivers/mmc/card/block.c |   33 -
>>>>  drivers/mmc/card/queue.c |    5 +
>>>>  drivers/mmc/core/bus.c   |   32 +++-
>>>>  drivers/mmc/core/core.c  |    8 +++-
>>>>  include/linux/mmc/card.h |    3 +++
>>>>  5 files changed, 74 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>>> index edc379e..83956fa 100644
>>>> --- a/drivers/mmc/card/block.c
>>>> +++ b/drivers/mmc/card/block.c
>>>> @@ -648,8 +648,15 @@ static int mmc_blk_cmd_recovery(struct mmc_card
>>>> *card, struct request *req,
>>>>        }
>>>>
>>>>        /* We couldn't get a response from the card.  Give up. */
>>>> -       if (err)
>>>> +       if (err) {
>>>> +               /*
>>>> +                * If the card didn't respond to status command,
>>>> +                * it is likely that card is gone. Flag it as removed,
>>>> +                * mmc_detect_change() cleans the rest.
>>>> +                */
>>>> +               mmc_card_set_removed(card);
>>>>                return ERR_ABORT;
>>>> +       }
>>>>
>>>>        /* Flag ECC errors */
>>>>        if ((status&  R1_CARD_ECC_FAILED) ||
>>>> @@ -1168,6 +1175,9 @@ static inline struct mmc_async_req
>>>> *mmc_blk_resend(struct mmc_card *card,
>>>>                                                   int disable_multi,
>>>>                                                   struct mmc_async_req
>>>> *areq)
>>>>  {
>>>> +       struct mmc_blk_data *md = mmc_get_drvdata(card);
>>>> +       struct request *req = mqrq->req;
>>>> +       int ret;
>>>>        /*
>>>>         * Release host after failure in case the host is needed
>>>>         * by someone else. For instance, if the card is removed the
>>>> @@ -1175,11 +1185,24 @@ static inline struct mmc_async_req
>>>> *mmc_blk_resend(struct mmc_card *card,
>>>>         */
>>>>        mmc_release_host(card->host);
>>>>        mmc_claim_host(card->host);
>>>> -
>>>> -       mmc_blk_rw_rq_prep(mqrq, card, disable_multi, mq);
>>>> -       return mmc_start_req(card->host, areq, NULL);
>>>> +       if (mmc_card_removed(card)) {
>>>> +               /*
>>>> +                * End the pending async request without starting
>>>> +                * it when card is removed.
>>>> +                */
>>>> +               spin_lock_irq(&md->lock);
>>>> +               req->cmd_flags |= REQ_QUIET;
>>>> +               do {
>>>> +                       ret = __blk_end_request(req,
>>>> +                                       -EIO, blk_rq_cur_bytes(req));
>>>> +               } while (ret);
>>>> +               spin_unlock_irq(&md->lock);
>>>> +               return NULL;
>>>> +       } else {
>>>> +               mmc_blk_rw_rq_prep(mqrq, card, disable_multi, mq);
>>>> +               return mmc_start_req(card->host, areq, NULL);
>>>> +       }
>>>
>>

Re: [PATCH] mmc: core: Kill block requests if card is removed

2011-11-24 Thread Sujit Reddy Thumma


On 11/24/2011 3:00 PM, Per Forlin wrote:

Hi David,

On Tue, Nov 22, 2011 at 9:18 PM, David Taylor  wrote:

Sujit Reddy Thumma  codeaurora.org>  writes:



Kill block requests when the host knows that the card is
removed from the slot and is sure that subsequent requests
are bound to fail. Do this silently so that the block
layer doesn't output unnecessary error messages.

This patch implements suggestion from Adrian Hunter,
http://thread.gmane.org/gmane.linux.kernel.mmc/2714/focus=3474

Signed-off-by: Sujit Reddy Thumma  codeaurora.org>
---
  drivers/mmc/card/queue.c |5 +
  drivers/mmc/core/bus.c   |2 ++
  include/linux/mmc/card.h |3 +++
  3 files changed, 10 insertions(+), 0 deletions(-)



Thanks, this patch worked nicely to control a seemingly endless series of
driver complaints when the SD card was removed during a transfer.

The OMAP4 I'm working on has hardware assisted detection of card removal, and
this patch looks like it will be sufficient.


What happens if you run "dd if=/dev/zero of=/dev/mmcblk0 bs=1M
count=1000" and during dd the card is ejected?
When I test this, the host is claimed until the dd has tried to
transfer all 1000MB.


That is true only when we are using >3.0 kernel.

--
Thanks,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2] mmc: Kill block requests if card is removed

2011-11-24 Thread Sujit Reddy Thumma


Hi Per,

On 11/22/2011 6:40 PM, Per Forlin wrote:

Hi Sujit,

On Tue, Nov 22, 2011 at 11:56 AM, Sujit Reddy Thumma
  wrote:

Kill block requests when the host knows that the card is
removed from the slot and is sure that subsequent requests
are bound to fail. Do this silently so that the block
layer doesn't output unnecessary error messages.

This patch implements suggestion from Adrian Hunter,
http://thread.gmane.org/gmane.linux.kernel.mmc/2714/focus=3474

Signed-off-by: Sujit Reddy Thumma

---
Changes in v2:
- Changed the implementation with further comments from Adrian
- Set the card removed flag in bus notifier callbacks
- This patch is now dependent on patch from Per Forlin:
http://thread.gmane.org/gmane.linux.kernel.mmc/11128/focus=11211
---
  drivers/mmc/card/block.c |   33 -
  drivers/mmc/card/queue.c |5 +
  drivers/mmc/core/bus.c   |   32 +++-
  drivers/mmc/core/core.c  |8 +++-
  include/linux/mmc/card.h |3 +++
  5 files changed, 74 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index edc379e..83956fa 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -648,8 +648,15 @@ static int mmc_blk_cmd_recovery(struct mmc_card *card, 
struct request *req,
}

/* We couldn't get a response from the card.  Give up. */
-   if (err)
+   if (err) {
+   /*
+* If the card didn't respond to status command,
+* it is likely that card is gone. Flag it as removed,
+* mmc_detect_change() cleans the rest.
+*/
+   mmc_card_set_removed(card);
return ERR_ABORT;
+   }

/* Flag ECC errors */
if ((status&  R1_CARD_ECC_FAILED) ||
@@ -1168,6 +1175,9 @@ static inline struct mmc_async_req *mmc_blk_resend(struct 
mmc_card *card,
   int disable_multi,
   struct mmc_async_req *areq)
  {
+   struct mmc_blk_data *md = mmc_get_drvdata(card);
+   struct request *req = mqrq->req;
+   int ret;
/*
 * Release host after failure in case the host is needed
 * by someone else. For instance, if the card is removed the
@@ -1175,11 +1185,24 @@ static inline struct mmc_async_req 
*mmc_blk_resend(struct mmc_card *card,
 */
mmc_release_host(card->host);
mmc_claim_host(card->host);
-
-   mmc_blk_rw_rq_prep(mqrq, card, disable_multi, mq);
-   return mmc_start_req(card->host, areq, NULL);
+   if (mmc_card_removed(card)) {
+   /*
+* End the pending async request without starting
+* it when card is removed.
+*/
+   spin_lock_irq(&md->lock);
+   req->cmd_flags |= REQ_QUIET;
+   do {
+   ret = __blk_end_request(req,
+   -EIO, blk_rq_cur_bytes(req));
+   } while (ret);
+   spin_unlock_irq(&md->lock);
+   return NULL;
+   } else {
+   mmc_blk_rw_rq_prep(mqrq, card, disable_multi, mq);
+   return mmc_start_req(card->host, areq, NULL);
+   }

mmc_blk_resend is only called to resend a request that has failed. If
the card is removed the request will still be issued, but when it
retries it will give up here.


Currently, we set the card_removed flag in two places:

1) If the host supports interrupt detection, mmc_detect_change() calls 
the bus detect method and we set card removed flag in bus notifier call 
back.


2) When there is a transfer going on (host is claimed by mmcqd) and the 
card is removed ungracefully, the driver sends an error code to the 
block layer. mmc_blk_cmd_recovery() tries to send CMD13 to the card 
which in this case fails and we set the card_removed flag.


So when we retry or send next async request we end it in 
mmc_blk_resend() and there will be no subsequent request to the driver 
since we are suppressing the requests in the queue layer.


So I guess there is no need to add further checks in the 
__mmc_start_req(), which could be redundant as there are no calls to the 
core layer from block layer once the card is found to be removed.


In summary the sequence I thought would be like this:
Case 1:
1) Transfer is going on (host claimed by mmcqd) and card is removed.
2) Driver is in middle of transaction, hence some kind of timeout error 
is returned.
3) mmc_blk_cmd_recovery(): Block layer does error checking and sets the 
card as removed.

4) Block layer then retries the request calling mmc_blk_resend().
5) Since the mmc_card_removed is true we end the request here and do not 
send any request to the core.


Case 2:
1) When there is no transfer going on (host->claimed = 0)
2) m

Re: [PATCH] mmc: core: Fix deadlock when the CONFIG_MMC_UNSAFE_RESUME is not defined

2011-11-22 Thread Sujit Reddy Thumma


On 11/22/2011 9:13 PM, Ulf Hansson wrote:

Sujit Reddy Thumma wrote:

mmc_suspend_host() tries to claim host during suspend
and release it only when the bus suspend operation is
compeleted. If CONFIG_MMC_UNSAFE_RESUME is defined and
the host is flagged as removable, mmc_suspend_host()
tries to remove the card. In this process, the file system
sync can get blocked trying to acquire host which is already
claimed by mmc_suspend_host() causing deadlock.

Fix this deadlock by releasing host before ->remove() is called.

Signed-off-by: Sujit Reddy Thumma 
---
drivers/mmc/core/core.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 271efea..8adbadf 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2338,6 +2338,7 @@ int mmc_suspend_host(struct mmc_host *host)
* We simply "remove" the card in this case.
* It will be redetected on resume.
*/
+ mmc_do_release_host(host);


Instead of having an "else" below move mmc_do_release_host to be done
before "if (err == -ENOSYS || !host->bus_ops->resume)"


Sure.




if (host->bus_ops->remove)
host->bus_ops->remove(host);
mmc_claim_host(host);
@@ -2346,8 +2347,9 @@ int mmc_suspend_host(struct mmc_host *host)
mmc_release_host(host);
host->pm_flags = 0;
err = 0;
+ } else {
+ mmc_do_release_host(host);
}


Remove else statement entirely.


Okay. Will do it.


Thanks,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2] mmc: core: Fix deadlock when the CONFIG_MMC_UNSAFE_RESUME is not defined

2011-11-22 Thread Sujit Reddy Thumma

mmc_suspend_host() tries to claim host during suspend
and release it only when the bus suspend operation is
compeleted. If CONFIG_MMC_UNSAFE_RESUME is defined and
the host is flagged as removable, mmc_suspend_host()
tries to remove the card. In this process, the file system
sync can get blocked trying to acquire host which is already
claimed by mmc_suspend_host() causing deadlock.

Fix this deadlock by releasing host before ->remove() is called.

Signed-off-by: Sujit Reddy Thumma 

---
Changes in v2:
- Addressed review comment from Ulf Hansson.
---
 drivers/mmc/core/core.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 271efea..950b97d 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2333,6 +2333,8 @@ int mmc_suspend_host(struct mmc_host *host)
mmc_poweroff_notify(host);
err = host->bus_ops->suspend(host);
}
+   mmc_do_release_host(host);
+
if (err == -ENOSYS || !host->bus_ops->resume) {
/*
 * We simply "remove" the card in this case.
@@ -2347,7 +2349,6 @@ int mmc_suspend_host(struct mmc_host *host)
host->pm_flags = 0;
err = 0;
}
-   mmc_do_release_host(host);
} else {
err = -EBUSY;
}
-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] mmc: core: Fix deadlock when the CONFIG_MMC_UNSAFE_RESUME is not defined

2011-11-22 Thread Sujit Reddy Thumma

mmc_suspend_host() tries to claim host during suspend
and release it only when the bus suspend operation is
compeleted. If CONFIG_MMC_UNSAFE_RESUME is defined and
the host is flagged as removable, mmc_suspend_host()
tries to remove the card. In this process, the file system
sync can get blocked trying to acquire host which is already
claimed by mmc_suspend_host() causing deadlock.

Fix this deadlock by releasing host before ->remove() is called.

Signed-off-by: Sujit Reddy Thumma 
---
 drivers/mmc/core/core.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 271efea..8adbadf 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2338,6 +2338,7 @@ int mmc_suspend_host(struct mmc_host *host)
 * We simply "remove" the card in this case.
 * It will be redetected on resume.
 */
+   mmc_do_release_host(host);
if (host->bus_ops->remove)
host->bus_ops->remove(host);
mmc_claim_host(host);
@@ -2346,8 +2347,9 @@ int mmc_suspend_host(struct mmc_host *host)
mmc_release_host(host);
host->pm_flags = 0;
err = 0;
+   } else {
+   mmc_do_release_host(host);
}
-   mmc_do_release_host(host);
} else {
err = -EBUSY;
}
-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2] mmc: Kill block requests if card is removed

2011-11-22 Thread Sujit Reddy Thumma

Kill block requests when the host knows that the card is
removed from the slot and is sure that subsequent requests
are bound to fail. Do this silently so that the block
layer doesn't output unnecessary error messages.

This patch implements suggestion from Adrian Hunter,
http://thread.gmane.org/gmane.linux.kernel.mmc/2714/focus=3474

Signed-off-by: Sujit Reddy Thumma 

---
Changes in v2:
- Changed the implementation with further comments from Adrian
- Set the card removed flag in bus notifier callbacks
- This patch is now dependent on patch from Per Forlin:
http://thread.gmane.org/gmane.linux.kernel.mmc/11128/focus=11211
---
 drivers/mmc/card/block.c |   33 -
 drivers/mmc/card/queue.c |5 +
 drivers/mmc/core/bus.c   |   32 +++-
 drivers/mmc/core/core.c  |8 +++-
 include/linux/mmc/card.h |3 +++
 5 files changed, 74 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index edc379e..83956fa 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -648,8 +648,15 @@ static int mmc_blk_cmd_recovery(struct mmc_card *card, 
struct request *req,
}
 
/* We couldn't get a response from the card.  Give up. */
-   if (err)
+   if (err) {
+   /*
+* If the card didn't respond to status command,
+* it is likely that card is gone. Flag it as removed,
+* mmc_detect_change() cleans the rest.
+*/
+   mmc_card_set_removed(card);
return ERR_ABORT;
+   }
 
/* Flag ECC errors */
if ((status & R1_CARD_ECC_FAILED) ||
@@ -1168,6 +1175,9 @@ static inline struct mmc_async_req *mmc_blk_resend(struct 
mmc_card *card,
   int disable_multi,
   struct mmc_async_req *areq)
 {
+   struct mmc_blk_data *md = mmc_get_drvdata(card);
+   struct request *req = mqrq->req;
+   int ret;
/*
 * Release host after failure in case the host is needed
 * by someone else. For instance, if the card is removed the
@@ -1175,11 +1185,24 @@ static inline struct mmc_async_req 
*mmc_blk_resend(struct mmc_card *card,
 */
mmc_release_host(card->host);
mmc_claim_host(card->host);
-
-   mmc_blk_rw_rq_prep(mqrq, card, disable_multi, mq);
-   return mmc_start_req(card->host, areq, NULL);
+   if (mmc_card_removed(card)) {
+   /*
+* End the pending async request without starting
+* it when card is removed.
+*/
+   spin_lock_irq(&md->lock);
+   req->cmd_flags |= REQ_QUIET;
+   do {
+   ret = __blk_end_request(req,
+   -EIO, blk_rq_cur_bytes(req));
+   } while (ret);
+   spin_unlock_irq(&md->lock);
+   return NULL;
+   } else {
+   mmc_blk_rw_rq_prep(mqrq, card, disable_multi, mq);
+   return mmc_start_req(card->host, areq, NULL);
+   }
 }
-
 static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 {
struct mmc_blk_data *md = mq->data;
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index dcad59c..2517547 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -29,6 +29,8 @@
  */
 static int mmc_prep_request(struct request_queue *q, struct request *req)
 {
+   struct mmc_queue *mq = q->queuedata;
+
/*
 * We only like normal block requests and discards.
 */
@@ -37,6 +39,9 @@ static int mmc_prep_request(struct request_queue *q, struct 
request *req)
return BLKPREP_KILL;
}
 
+   if (mq && mmc_card_removed(mq->card))
+   return BLKPREP_KILL;
+
req->cmd_flags |= REQ_DONTPREP;
 
return BLKPREP_OK;
diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 5639fdf..ca6e07a 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -102,6 +102,25 @@ mmc_bus_uevent(struct device *dev, struct kobj_uevent_env 
*env)
return retval;
 }
 
+static int mmc_bus_notify(struct notifier_block *nb, unsigned long action,
+   void *data)
+{
+   struct mmc_card *card = mmc_dev_to_card((struct device *) data);
+
+   switch (action) {
+   case BUS_NOTIFY_DEL_DEVICE:
+   mmc_card_set_removed(card);
+   break;
+   default:
+   break;
+   }
+   return 0;
+}
+
+static struct notifier_block mmc_bus_nb = {
+   .notifier_call = mmc_bus_notify,
+};
+
 static int mmc_bus_probe(struct device *dev)
 {
struct mmc_driver *drv = to_mmc_driver(dev->driver);
@@ -191,11 +210,22 @@ static struct

Re: [PATCH] mmc: block: release host in case of error

2011-11-16 Thread Sujit Reddy Thumma


On 11/14/2011 4:42 PM, Per Forlin wrote:

Host is claimed as long as there are requests in the block queue
and all request are completed successfully. If an error occur release
the host in case someone else needs to claim it, for instance if the card
is removed during a transfer.

Signed-off-by: Per Forlin
---
  drivers/mmc/card/block.c |   37 +
  1 files changed, 29 insertions(+), 8 deletions(-)


Tested-by: Sujit Reddy Thumma 


Thanks
Sujit

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: mmc_test: align max_seg_size

2011-11-16 Thread Sujit Reddy Thumma


On 11/14/2011 4:34 PM, Per Forlin wrote:

If max_seg_size is unaligned, mmc_test_map_sg() may create sg element
sizes that are not aligned with 512 byte. Fix, align max_seg_size at
mmc_test_area_init().

Signed-off-by: Per Forlin
---
  drivers/mmc/card/mmc_test.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
index b038c4a..5848998 100644
--- a/drivers/mmc/card/mmc_test.c
+++ b/drivers/mmc/card/mmc_test.c
@@ -1581,6 +1581,7 @@ static int mmc_test_area_init(struct mmc_test_card *test, 
int erase, int fill)

t->max_segs = test->card->host->max_segs;
t->max_seg_sz = test->card->host->max_seg_size;
+   t->max_seg_sz -= t->max_seg_sz % 512;


Shouldn't we align this to host->max_blk_size supported by controller? I 
guess there are some extended capacity cards which claim to support 
4096bytes as block size. I am not sure if any card/controller supports 
this mode yet.




t->max_tfr = t->max_sz;
if (t->max_tfr>>  9>  test->card->host->max_blk_count)



--
Thanks,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Query] mmc/sdhci: Suspend hangs if card is inserted at suspend.

2011-11-15 Thread Sujit Reddy Thumma


Hi Ulf,

I see similar issue with your patch in 3.2 "mmc: core: Prevent too long 
response times for suspend".


if (mmc_try_claim_host(host)) {

/* if CONFIG_MMC_UNSAFE_RESUME is not set, remove() callback would get 
blocked until mmcqd gets mmc_claim_host() causing deadlock here */

if (host->bus_ops->remove)
host->bus_ops->remove(host);

} else {
err = -EBUSY;
}
}

Proabably, we can do something like this:

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 5278ffb..0177d4a 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2309,6 +2309,7 @@ int mmc_suspend_host(struct mmc_host *host)
 * We simply "remove" the card in this 
case.

 * It will be redetected on resume.
 */
+   mmc_do_release_host(host);
if (host->bus_ops->remove)
host->bus_ops->remove(host);
mmc_claim_host(host);
@@ -2317,8 +2318,9 @@ int mmc_suspend_host(struct mmc_host *host)
mmc_release_host(host);
host->pm_flags = 0;
err = 0;
+   } else {
+   mmc_do_release_host(host);
}
-   mmc_do_release_host(host);
} else {
err = -EBUSY;
}

Please let me know if you agree. I can post a patch for this.


Thanks
Sujit

On 11/15/2011 12:27 PM, Dong, Chuanxiao wrote:

Hi Viresh,
I also meet the hang issue when suspending a system with a mounted SD card. 
Actually it caused by a dead lock of mmc_claim_host. Maybe we faced the same 
issue.
So may be the two mails in the attachment can help you.

Thanks
Chuanxiao


-Original Message-
From: linux-mmc-ow...@vger.kernel.org
[mailto:linux-mmc-ow...@vger.kernel.org] On Behalf Of Viresh Kumar
Sent: Tuesday, November 15, 2011 1:47 PM
To: Chris Ball
Cc: linux-mmc@vger.kernel.org; Shiraz HASHIM; Armando VISCONTI
Subject: Re: [Query] mmc/sdhci: Suspend hangs if card is inserted at suspend.

On 11/12/2011 8:54 AM, Chris Ball wrote:

On Fri, Nov 11 2011, Viresh Kumar wrote:

Controller specific suspend/resume are still not called, so i think it might
be related to core code. I am using 2.6.37 kernel version.

Any help on this?

Please try to reproduce on 3.1 and let us know if it's present there.


Hi Chris,

Support for SPEAr13xx is not present on 3.1 and it will take some time to
move support there. I have a bit more info which might able you to guide us:

file: drivers/mmc/card/queue.c

static int mmc_queue_thread(void *d)
{
...
do {
req = blk_fetch_request(q);
...
if (!req) {
if (kthread_should_stop()) {
set_current_state(TASK_RUNNING);
break;
}
up(&mq->thread_sem);
schedule();
down(&mq->thread_sem);
continue;
}
set_current_state(TASK_RUNNING);
mq->issue_fn(mq, req);
} while (1);
...
}

- In normal cases, i.e. when we are able to stop this thread,
   req is returned as NULL from blk_fetch_request()
- Suspend hang only occurs when the card is mounted.
- When suspend hangs, req is not returned as NULL and the card is
   already removed by kernel and so we never check kthread_should_stop().
   So it hangs.

--
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



--
Thanks & Regards,
Sujit Reddy Thumma

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3] mmc: core: Use delayed work in clock gating framework

2011-11-14 Thread Sujit Reddy Thumma


Hi Chris,

On 11/14/2011 6:45 PM, Chris Ball wrote:

Hi Sujit,

On Mon, Nov 14 2011, Sujit Reddy Thumma wrote:

Current clock gating framework disables the MCI clock as soon as the
request is completed and enables it when a request arrives. This aggressive
clock gating framework, when enabled, cause following issues:

When there are back-to-back requests from the Queue layer, we unnecessarily
end up disabling and enabling the clocks between these requests since 8MCLK
clock cycles is a very short duration compared to the time delay between
back to back requests reaching the MMC layer. This overhead can effect the
overall performance depending on how long the clock enable and disable
calls take which is platform dependent. For example on some platforms we
can have clock control not on the local processor, but on a different
subsystem and the time taken to perform the clock enable/disable can add
significant overhead.

Also if the host controller driver decides to disable the host clock too
when mmc_set_ios function is called with ios.clock=0, it adds additional
delay and it is highly possible that the next request had already arrived
and unnecessarily blocked in enabling the clocks. This is seen frequently
when the processor is executing at high speeds and in multi-core platforms
thus reduces the overall throughput compared to if clock gating is
disabled.

Fix this by delaying turning off the clocks by posting request on
delayed workqueue. Also cancel the unscheduled pending work, if any,
when there is access to card.

sysfs entry is provided to tune the delay as needed, default
value set to 200ms.

Signed-off-by: Sujit Reddy Thumma
Acked-by: Linus Walleij


error: patch failed: drivers/mmc/core/host.c:53
error: drivers/mmc/core/host.c: patch does not apply
error: patch failed: include/linux/mmc/host.h:253
error: include/linux/mmc/host.h: patch does not apply

Please could you resend against current mmc-next?


I see that the patch is already applied on mmc-next, hence the conflicts.

http://git.kernel.org/?p=linux/kernel/git/cjb/mmc.git;a=commit;h=0987075c3c285ba92f93464f7a882515d4a054d1

Can we rebase the tree and apply V3?



Thanks,

- Chris.



--
Thanks,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: core: Kill block requests if card is removed

2011-11-14 Thread Sujit Reddy Thumma


On 11/14/2011 1:54 PM, Per Forlin wrote:

On Mon, Nov 14, 2011 at 8:52 AM, Per Forlin  wrote:

On Mon, Nov 14, 2011 at 5:19 AM, Sujit Reddy Thumma
  wrote:

On 11/10/2011 7:50 PM, Per Forlin wrote:


On Thu, Nov 10, 2011 at 10:35 AM, Adrian Hunter
  wrote:


On 10/11/11 06:02, Sujit Reddy Thumma wrote:


Hi,


Hi Adrian,

On Wed, Nov 9, 2011 at 10:34 AM, Adrian Hunter
wrote:


On 09/11/11 06:31, Sujit Reddy Thumma wrote:


Kill block requests when the host knows that the card is
removed from the slot and is sure that subsequent requests
are bound to fail. Do this silently so that the block
layer doesn't output unnecessary error messages.

This patch implements suggestion from Adrian Hunter,
http://thread.gmane.org/gmane.linux.kernel.mmc/2714/focus=3474

Signed-off-by: Sujit Reddy Thumma
---



It is safer to have zero initialisations so I suggest inverting
the meaning of the state bit i.e.


Makes sense. Will take care in V2.



#define MMC_STATE_CARD_GONE (1<<7)  /* card removed from
the
slot */


Also it would be nice is a request did not start if the card is
gone.  For the non-async case, that is easy:


As far as I understand Sujit's patch it will stop new requests from
being issued, blk_fetch_request(q) returns NULL.


Yes, Per you are right. The ongoing requests will fail at the controller
driver layer and only the new requests coming to MMC queue layer will be
blocked.

Adrian's suggestion is to stop all the requests reaching host controller
layer by mmc core. This seems to be a good approach but the problem here
is
the host driver should inform mmc core that card is gone.

Adrian, do you agree if we need to change all the host drivers to set
the
MMC_STATE_CARD_GONE flag as soon as the card detect irq handler detects
the
card as removed?


Typically a card detect interrupt queues a rescan which in turn will have
to wait to claim the host.  In the meantime, in the async case, the block
driver will not release the host until the queue is empty.


Here is a patch that let async-mmc release host and reclaim it in case of
error.
When the host is released mmc_rescan will claim the host and run.

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index cf73877..8952e63 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1209,6 +1209,28 @@ static int mmc_blk_cmd_err(struct mmc_blk_data
*md, struct mmc_card *card,
return ret;
  }

+/*
+ * This function should be called to resend a request after failure.
+ * Prepares and starts the request.
+ */
+static inline struct mmc_async_req *mmc_blk_resend(struct mmc_card *card,
+  struct mmc_queue *mq,
+  struct mmc_queue_req
*mqrq,
+  int disable_multi,
+  struct mmc_async_req
*areq)
+{
+   /*
+* Release host after failure in case the host is needed
+* by someone else. For instance, if the card is removed the
+* worker thread needs to claim the host in order to do
mmc_rescan.
+*/
+   mmc_release_host(card->host);
+   mmc_claim_host(card->host);
+
+   mmc_blk_rw_rq_prep(mqrq, card, disable_multi, mq);
+   return mmc_start_req(card->host, areq, NULL);
+}
+
  static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
  {
struct mmc_blk_data *md = mq->data;
@@ -1308,14 +1330,14 @@ static int mmc_blk_issue_rw_rq(struct
mmc_queue *mq, struct request *rqc)
break;
}

-   if (ret) {
+   if (ret)
/*
 * In case of a incomplete request
 * prepare it again and resend.
 */
-   mmc_blk_rw_rq_prep(mq_rq, card, disable_multi,
mq);
-   mmc_start_req(card->host,&mq_rq->mmc_active,
NULL);
-   }
+   mmc_blk_prep_start(card, mq, mq_rq, disable_multi,
+&mq_rq->mmc_active);
+


:%s/mmc_blk_prep_start/mmc_blk_resend


I'll update.


} while (ret);

return 1;
@@ -1327,10 +1349,9 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
*mq, struct request *rqc)
spin_unlock_irq(&md->lock);

   start_new_req:
-   if (rqc) {
-   mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
-   mmc_start_req(card->host,&mq->mqrq_cur->mmc_active, NULL);
-   }
+   if (rqc)
+   mmc_blk_prep_start(card, mq, mq->mqrq_cur, 0,
+&mq->mqrq_cur->mmc_active);

return 0;
  }


Thanks Per. This looks good. Can we split this into a different patch?


Yes I agree. My intention was to treat this as a separate patch.
I'll post it.


-



  The block

[PATCH V3] mmc: core: Use delayed work in clock gating framework

2011-11-14 Thread Sujit Reddy Thumma

Current clock gating framework disables the MCI clock as soon as the
request is completed and enables it when a request arrives. This aggressive
clock gating framework, when enabled, cause following issues:

When there are back-to-back requests from the Queue layer, we unnecessarily
end up disabling and enabling the clocks between these requests since 8MCLK
clock cycles is a very short duration compared to the time delay between
back to back requests reaching the MMC layer. This overhead can effect the
overall performance depending on how long the clock enable and disable
calls take which is platform dependent. For example on some platforms we
can have clock control not on the local processor, but on a different
subsystem and the time taken to perform the clock enable/disable can add
significant overhead.

Also if the host controller driver decides to disable the host clock too
when mmc_set_ios function is called with ios.clock=0, it adds additional
delay and it is highly possible that the next request had already arrived
and unnecessarily blocked in enabling the clocks. This is seen frequently
when the processor is executing at high speeds and in multi-core platforms
thus reduces the overall throughput compared to if clock gating is
disabled.

Fix this by delaying turning off the clocks by posting request on
delayed workqueue. Also cancel the unscheduled pending work, if any,
when there is access to card.

sysfs entry is provided to tune the delay as needed, default
value set to 200ms.

Signed-off-by: Sujit Reddy Thumma 
Acked-by: Linus Walleij 

---
Changes in v3:
- Documentation for sysfs entry is added.

Changes in v2:
- support for tuning delay via sysfs entry.
---
 Documentation/mmc/mmc-dev-attrs.txt |   10 ++
 drivers/mmc/core/host.c |   57 --
 include/linux/mmc/host.h|4 ++-
 3 files changed, 66 insertions(+), 5 deletions(-)

diff --git a/Documentation/mmc/mmc-dev-attrs.txt 
b/Documentation/mmc/mmc-dev-attrs.txt
index 8898a95..b024556 100644
--- a/Documentation/mmc/mmc-dev-attrs.txt
+++ b/Documentation/mmc/mmc-dev-attrs.txt
@@ -64,3 +64,13 @@ Note on Erase Size and Preferred Erase Size:
size specified by the card.
 
"preferred_erase_size" is in bytes.
+
+SD/MMC/SDIO Clock Gating Attribute
+==
+
+Read and write access is provided to following attribute.
+This attribute appears only if CONFIG_MMC_CLKGATE is enabled.
+
+   clkgate_delay   Tune the clock gating delay with desired value in milli 
seconds.
+
+echo  > /sys/class/mmc_host/mmcX/clkgate_delay
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index ca2e4f5..ba4cc5d 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -53,6 +53,31 @@ static DEFINE_IDR(mmc_host_idr);
 static DEFINE_SPINLOCK(mmc_host_lock);
 
 #ifdef CONFIG_MMC_CLKGATE
+static ssize_t clkgate_delay_show(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   struct mmc_host *host = cls_dev_to_mmc_host(dev);
+   return snprintf(buf, PAGE_SIZE, "%lu millisecs\n",
+   host->clkgate_delay);
+}
+
+static ssize_t clkgate_delay_store(struct device *dev,
+   struct device_attribute *attr, const char *buf, size_t count)
+{
+   struct mmc_host *host = cls_dev_to_mmc_host(dev);
+   unsigned long flags, value;
+
+   if (kstrtoul(buf, 0, &value))
+   return -EINVAL;
+
+   spin_lock_irqsave(&host->clk_lock, flags);
+   host->clkgate_delay = value;
+   spin_unlock_irqrestore(&host->clk_lock, flags);
+
+   pr_info("%s: clock gate delay set to %lu ms\n",
+   mmc_hostname(host), value);
+   return count;
+}
 
 /*
  * Enabling clock gating will make the core call out to the host
@@ -113,7 +138,7 @@ static void mmc_host_clk_gate_delayed(struct mmc_host *host)
 static void mmc_host_clk_gate_work(struct work_struct *work)
 {
struct mmc_host *host = container_of(work, struct mmc_host,
- clk_gate_work);
+ clk_gate_work.work);
 
mmc_host_clk_gate_delayed(host);
 }
@@ -130,6 +155,8 @@ void mmc_host_clk_hold(struct mmc_host *host)
 {
unsigned long flags;
 
+   /* cancel any clock gating work scheduled by mmc_host_clk_release() */
+   cancel_delayed_work_sync(&host->clk_gate_work);
mutex_lock(&host->clk_gate_mutex);
spin_lock_irqsave(&host->clk_lock, flags);
if (host->clk_gated) {
@@ -179,7 +206,8 @@ void mmc_host_clk_release(struct mmc_host *host)
host->clk_requests--;
if (mmc_host_may_gate_card(host->card) &&
!host->clk_requests)
-   queue_work(system_nrt_wq, &host->clk_gate_work);
+   queue_delayed_work(

Re: [PATCH] mmc: core: Kill block requests if card is removed

2011-11-13 Thread Sujit Reddy Thumma


On 11/10/2011 7:50 PM, Per Forlin wrote:

On Thu, Nov 10, 2011 at 10:35 AM, Adrian Hunter  wrote:

On 10/11/11 06:02, Sujit Reddy Thumma wrote:

Hi,

Hi Adrian,

On Wed, Nov 9, 2011 at 10:34 AM, Adrian Hunter
wrote:

On 09/11/11 06:31, Sujit Reddy Thumma wrote:

Kill block requests when the host knows that the card is
removed from the slot and is sure that subsequent requests
are bound to fail. Do this silently so that the block
layer doesn't output unnecessary error messages.

This patch implements suggestion from Adrian Hunter,
http://thread.gmane.org/gmane.linux.kernel.mmc/2714/focus=3474

Signed-off-by: Sujit Reddy Thumma
---



It is safer to have zero initialisations so I suggest inverting
the meaning of the state bit i.e.


Makes sense. Will take care in V2.



#define MMC_STATE_CARD_GONE (1<<7)  /* card removed from the
slot */


Also it would be nice is a request did not start if the card is
gone.  For the non-async case, that is easy:


As far as I understand Sujit's patch it will stop new requests from
being issued, blk_fetch_request(q) returns NULL.


Yes, Per you are right. The ongoing requests will fail at the controller
driver layer and only the new requests coming to MMC queue layer will be
blocked.

Adrian's suggestion is to stop all the requests reaching host controller
layer by mmc core. This seems to be a good approach but the problem here is
the host driver should inform mmc core that card is gone.

Adrian, do you agree if we need to change all the host drivers to set the
MMC_STATE_CARD_GONE flag as soon as the card detect irq handler detects the
card as removed?


Typically a card detect interrupt queues a rescan which in turn will have
to wait to claim the host.  In the meantime, in the async case, the block
driver will not release the host until the queue is empty.

Here is a patch that let async-mmc release host and reclaim it in case of error.
When the host is released mmc_rescan will claim the host and run.

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index cf73877..8952e63 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1209,6 +1209,28 @@ static int mmc_blk_cmd_err(struct mmc_blk_data
*md, struct mmc_card *card,
return ret;
  }

+/*
+ * This function should be called to resend a request after failure.
+ * Prepares and starts the request.
+ */
+static inline struct mmc_async_req *mmc_blk_resend(struct mmc_card *card,
+  struct mmc_queue *mq,
+  struct mmc_queue_req *mqrq,
+  int disable_multi,
+  struct mmc_async_req *areq)
+{
+   /*
+* Release host after failure in case the host is needed
+* by someone else. For instance, if the card is removed the
+* worker thread needs to claim the host in order to do mmc_rescan.
+*/
+   mmc_release_host(card->host);
+   mmc_claim_host(card->host);
+
+   mmc_blk_rw_rq_prep(mqrq, card, disable_multi, mq);
+   return mmc_start_req(card->host, areq, NULL);
+}
+
  static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
  {
struct mmc_blk_data *md = mq->data;
@@ -1308,14 +1330,14 @@ static int mmc_blk_issue_rw_rq(struct
mmc_queue *mq, struct request *rqc)
break;
}

-   if (ret) {
+   if (ret)
/*
 * In case of a incomplete request
 * prepare it again and resend.
 */
-   mmc_blk_rw_rq_prep(mq_rq, card, disable_multi, mq);
-   mmc_start_req(card->host,&mq_rq->mmc_active, NULL);
-   }
+   mmc_blk_prep_start(card, mq, mq_rq, disable_multi,
+   &mq_rq->mmc_active);
+


:%s/mmc_blk_prep_start/mmc_blk_resend


} while (ret);

return 1;
@@ -1327,10 +1349,9 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
*mq, struct request *rqc)
spin_unlock_irq(&md->lock);

   start_new_req:
-   if (rqc) {
-   mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
-   mmc_start_req(card->host,&mq->mqrq_cur->mmc_active, NULL);
-   }
+   if (rqc)
+   mmc_blk_prep_start(card, mq, mq->mqrq_cur, 0,
+   &mq->mqrq_cur->mmc_active);

return 0;
  }


Thanks Per. This looks good. Can we split this into a different patch?


-



  The block
driver will see errors and will use a send status command which will fail
so the request will be aborted, but the next request will be started
anyway.

There are two problems:

1. When do we reliabl

Re: [PATCH] mmc: core: Kill block requests if card is removed

2011-11-09 Thread Sujit Reddy Thumma


Hi Per,

On 11/10/2011 3:17 AM, Per Forlin wrote:

Hi Sujit,

On Wed, Nov 9, 2011 at 5:31 AM, Sujit Reddy Thumma
  wrote:

Kill block requests when the host knows that the card is
removed from the slot and is sure that subsequent requests
are bound to fail. Do this silently so that the block
layer doesn't output unnecessary error messages.

This patch implements suggestion from Adrian Hunter,
http://thread.gmane.org/gmane.linux.kernel.mmc/2714/focus=3474

Signed-off-by: Sujit Reddy Thumma
---
  drivers/mmc/card/queue.c |5 +
  drivers/mmc/core/bus.c   |2 ++
  include/linux/mmc/card.h |3 +++
  3 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index dcad59c..f8a3298 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -29,6 +29,8 @@
  */
  static int mmc_prep_request(struct request_queue *q, struct request *req)
  {
+   struct mmc_queue *mq = q->queuedata;
+
/*
 * We only like normal block requests and discards.
 */
@@ -37,6 +39,9 @@ static int mmc_prep_request(struct request_queue *q, struct 
request *req)
return BLKPREP_KILL;
}

+   if (mq&&  mq->card&&  !mmc_card_inserted(mq->card))

I guess the card is not freed until all pending requests have been
flushed? mq->card will be valid as long as the queue is active.


Agreed. This is a redundant check, will remove it in v2.



Another way to detect card removal is to subscribe for
"BUS_NOTIFY_DEL_DEVICE" mmc card device.


Thanks, this sounds good, for the current v1 patch.
I have a concern about this when we take Adrian's suggestion. If we want 
to set the card gone flag as soon as the card is removed, then we can 
stop any new block request. Registering for BUS_NOTIFY_DEL_DEVICE only 
stops sync requests issued in device_del().





+   return BLKPREP_KILL;
+
req->cmd_flags |= REQ_DONTPREP;

return BLKPREP_OK;
diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 46b6e84..ea3be5d 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -308,6 +308,7 @@ int mmc_add_card(struct mmc_card *card)
mmc_card_ddr_mode(card) ? "DDR " : "",
type, card->rca);
}
+   mmc_card_set_inserted(card);

If the card-alloction is freed when the card is removed. Is it
necessary to set this bit for the new allocated card? Or could this be
the same card allocation?

Adrian's suggestion, "It is safer to have zero initialisations so I 
suggest inverting the meaning of the state bit," I guess, answer your 
question. We set the card gone flag when the card is removed i.e., 
either in mmc_remove_card() or host driver's card detect irq handler and 
while card allocation is freed it is cleared anyways.


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: core: Kill block requests if card is removed

2011-11-09 Thread Sujit Reddy Thumma


On 11/10/2011 3:35 AM, Per Forlin wrote:

Hi Adrian,


diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 5278ffb..91d7721 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -259,7 +259,7 @@ static void mmc_wait_for_req_done(struct mmc_host *host,
wait_for_completion(&mrq->completion);

cmd = mrq->cmd;
-   if (!cmd->error || !cmd->retries)
+   if (!cmd->error || !cmd->retries || mmc_card_gone(host->card))

host->card will be NULL
static void mmc_remove(struct mmc_host *host)
{
BUG_ON(!host);
BUG_ON(!host->card);

mmc_remove_card(host->card);
host->card = NULL;
}
card is not freed until later.

Please ignore this part. I jumped to conclusions. I had another look
and there can't be any incoming requests when host->card is NULL.
I need to study device_del() further, in order to understand the details.


There can be incoming requests when the host->card is NULL. This happens 
when we are detecting the card for the first time. That is, in 
mmc_rescan() we send all the initialization commands with host->card 
being NULL.


We can do something like this:
#define mmc_card_gone(c) (c && ((c)->state & MMC_STATE_CARD_GONE))




Regards,
Per



--
Thanks & Regards,
Sujit Reddy Thumma

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: core: Kill block requests if card is removed

2011-11-09 Thread Sujit Reddy Thumma


Hi,

Hi Adrian,

On Wed, Nov 9, 2011 at 10:34 AM, Adrian Hunter  wrote:

On 09/11/11 06:31, Sujit Reddy Thumma wrote:

Kill block requests when the host knows that the card is
removed from the slot and is sure that subsequent requests
are bound to fail. Do this silently so that the block
layer doesn't output unnecessary error messages.

This patch implements suggestion from Adrian Hunter,
http://thread.gmane.org/gmane.linux.kernel.mmc/2714/focus=3474

Signed-off-by: Sujit Reddy Thumma
---



It is safer to have zero initialisations so I suggest inverting
the meaning of the state bit i.e.


Makes sense. Will take care in V2.



#define MMC_STATE_CARD_GONE (1<<7)  /* card removed from the slot */


Also it would be nice is a request did not start if the card is
gone.  For the non-async case, that is easy:


As far as I understand Sujit's patch it will stop new requests from
being issued, blk_fetch_request(q) returns NULL.


Yes, Per you are right. The ongoing requests will fail at the controller 
driver layer and only the new requests coming to MMC queue layer will be 
blocked.


Adrian's suggestion is to stop all the requests reaching host controller 
layer by mmc core. This seems to be a good approach but the problem here 
is the host driver should inform mmc core that card is gone.


Adrian, do you agree if we need to change all the host drivers to set 
the MMC_STATE_CARD_GONE flag as soon as the card detect irq handler 
detects the card as removed?





diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 5278ffb..91d7721 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -259,7 +259,7 @@ static void mmc_wait_for_req_done(struct mmc_host *host,
wait_for_completion(&mrq->completion);

cmd = mrq->cmd;
-   if (!cmd->error || !cmd->retries)
+   if (!cmd->error || !cmd->retries || mmc_card_gone(host->card))

host->card will be NULL


Yes, the host->card will be NULL for some requests. Will take care of it.


static void mmc_remove(struct mmc_host *host)
{
BUG_ON(!host);
BUG_ON(!host->card);

mmc_remove_card(host->card);
host->card = NULL;
}
card is not freed until later.


break;

pr_debug("%s: req failed (CMD%u): %d, retrying...\n",
@@ -374,6 +374,10 @@ EXPORT_SYMBOL(mmc_start_req);
  */
  void mmc_wait_for_req(struct mmc_host *host, struct mmc_request *mrq)
  {
+   if (mmc_card_gone(host->card)) {
+   mrq->cmd->error = -ENODEV;
+   return;
+   }


Looks good. Just for clarification, some host controller drivers return 
-ENOMEDIUM in host->ops->request() when they see the card is gone. Do 
you think we can remove this now from host controller drivers?




__mmc_start_req(host, mrq);
mmc_wait_for_req_done(host, mrq);
  }



The async case is harder...


I can help out with the non-async code if we go for your proposal.


I will check the possibilities.






  drivers/mmc/card/queue.c |5 +
  drivers/mmc/core/bus.c   |2 ++
  include/linux/mmc/card.h |3 +++
  3 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index dcad59c..f8a3298 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -29,6 +29,8 @@
   */
  static int mmc_prep_request(struct request_queue *q, struct request *req)
  {
+ struct mmc_queue *mq = q->queuedata;
+
   /*
* We only like normal block requests and discards.
*/
@@ -37,6 +39,9 @@ static int mmc_prep_request(struct request_queue *q, struct 
request *req)
   return BLKPREP_KILL;
   }

+ if (mq&&  mq->card&&  !mmc_card_inserted(mq->card))
+ return BLKPREP_KILL;
+
   req->cmd_flags |= REQ_DONTPREP;

   return BLKPREP_OK;
diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 46b6e84..ea3be5d 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -308,6 +308,7 @@ int mmc_add_card(struct mmc_card *card)
   mmc_card_ddr_mode(card) ? "DDR " : "",
   type, card->rca);
   }
+ mmc_card_set_inserted(card);

  #ifdef CONFIG_DEBUG_FS
   mmc_add_card_debugfs(card);
@@ -340,6 +341,7 @@ void mmc_remove_card(struct mmc_card *card)
   pr_info("%s: card %04x removed\n",
   mmc_hostname(card->host), card->rca);
   }
+ card->state&= ~MMC_STATE_INSERTED;
   device_del(&card->dev);
   }

diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index b7a8cb1..be4125a 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -209,6 +209,7 @@ struct mmc_card {
  #define MMC_STATE_HIGHSP

Re: [PATCH V2] mmc: core: Use delayed work in clock gating framework

2011-11-08 Thread Sujit Reddy Thumma


On 10/27/2011 12:44 PM, Sujit Reddy Thumma wrote:

Current clock gating framework disables the MCI clock as soon as the
request is completed and enables it when a request arrives. This aggressive
clock gating framework, when enabled, cause following issues:

When there are back-to-back requests from the Queue layer, we unnecessarily
end up disabling and enabling the clocks between these requests since 8MCLK
clock cycles is a very short duration compared to the time delay between
back to back requests reaching the MMC layer. This overhead can effect the
overall performance depending on how long the clock enable and disable
calls take which is platform dependent. For example on some platforms we
can have clock control not on the local processor, but on a different
subsystem and the time taken to perform the clock enable/disable can add
significant overhead.

Also if the host controller driver decides to disable the host clock too
when mmc_set_ios function is called with ios.clock=0, it adds additional
delay and it is highly possible that the next request had already arrived
and unnecessarily blocked in enabling the clocks. This is seen frequently
when the processor is executing at high speeds and in multi-core platforms
thus reduces the overall throughput compared to if clock gating is
disabled.

Fix this by delaying turning off the clocks by posting request on
delayed workqueue. Also cancel the unscheduled pending work, if any,
when there is access to card.

sysfs entry is provided to tune the delay as needed, default
value set to 200ms.

Signed-off-by: Sujit Reddy Thumma

---
Changes in v2:
- support for tuning delay via sysfs entry.


Linus/Chris, any comments on this V2 version?


---
  drivers/mmc/core/host.c  |   57 ++---
  include/linux/mmc/host.h |4 ++-
  2 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index ca2e4f5..ba4cc5d 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -53,6 +53,31 @@ static DEFINE_IDR(mmc_host_idr);
  static DEFINE_SPINLOCK(mmc_host_lock);

  #ifdef CONFIG_MMC_CLKGATE
+static ssize_t clkgate_delay_show(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   struct mmc_host *host = cls_dev_to_mmc_host(dev);
+   return snprintf(buf, PAGE_SIZE, "%lu millisecs\n",
+   host->clkgate_delay);
+}
+
+static ssize_t clkgate_delay_store(struct device *dev,
+   struct device_attribute *attr, const char *buf, size_t count)
+{
+   struct mmc_host *host = cls_dev_to_mmc_host(dev);
+   unsigned long flags, value;
+
+   if (kstrtoul(buf, 0,&value))
+   return -EINVAL;
+
+   spin_lock_irqsave(&host->clk_lock, flags);
+   host->clkgate_delay = value;
+   spin_unlock_irqrestore(&host->clk_lock, flags);
+
+   pr_info("%s: clock gate delay set to %lu ms\n",
+   mmc_hostname(host), value);
+   return count;
+}

  /*
   * Enabling clock gating will make the core call out to the host
@@ -113,7 +138,7 @@ static void mmc_host_clk_gate_delayed(struct mmc_host *host)
  static void mmc_host_clk_gate_work(struct work_struct *work)
  {
struct mmc_host *host = container_of(work, struct mmc_host,
- clk_gate_work);
+ clk_gate_work.work);

mmc_host_clk_gate_delayed(host);
  }
@@ -130,6 +155,8 @@ void mmc_host_clk_hold(struct mmc_host *host)
  {
unsigned long flags;

+   /* cancel any clock gating work scheduled by mmc_host_clk_release() */
+   cancel_delayed_work_sync(&host->clk_gate_work);
mutex_lock(&host->clk_gate_mutex);
spin_lock_irqsave(&host->clk_lock, flags);
if (host->clk_gated) {
@@ -179,7 +206,8 @@ void mmc_host_clk_release(struct mmc_host *host)
host->clk_requests--;
if (mmc_host_may_gate_card(host->card)&&
!host->clk_requests)
-   queue_work(system_nrt_wq,&host->clk_gate_work);
+   queue_delayed_work(system_nrt_wq,&host->clk_gate_work,
+   msecs_to_jiffies(host->clkgate_delay));
spin_unlock_irqrestore(&host->clk_lock, flags);
  }

@@ -212,8 +240,13 @@ static inline void mmc_host_clk_init(struct mmc_host *host)
host->clk_requests = 0;
/* Hold MCI clock for 8 cycles by default */
host->clk_delay = 8;
+   /*
+* Default clock gating delay is 200ms.
+* This value can be tuned by writing into sysfs entry.
+*/
+   host->clkgate_delay = 200;
host->clk_gated = false;
-   INIT_WORK(&host->clk_gate_work, mmc_host_clk_gate_work);
+   INIT_DELAYED_WORK(&host->clk_gate_work, mmc_host_clk_gate_work);
spin_loc

[PATCH] mmc: core: Kill block requests if card is removed

2011-11-08 Thread Sujit Reddy Thumma

Kill block requests when the host knows that the card is
removed from the slot and is sure that subsequent requests
are bound to fail. Do this silently so that the block
layer doesn't output unnecessary error messages.

This patch implements suggestion from Adrian Hunter,
http://thread.gmane.org/gmane.linux.kernel.mmc/2714/focus=3474

Signed-off-by: Sujit Reddy Thumma 
---
 drivers/mmc/card/queue.c |5 +
 drivers/mmc/core/bus.c   |2 ++
 include/linux/mmc/card.h |3 +++
 3 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index dcad59c..f8a3298 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -29,6 +29,8 @@
  */
 static int mmc_prep_request(struct request_queue *q, struct request *req)
 {
+   struct mmc_queue *mq = q->queuedata;
+
/*
 * We only like normal block requests and discards.
 */
@@ -37,6 +39,9 @@ static int mmc_prep_request(struct request_queue *q, struct 
request *req)
return BLKPREP_KILL;
}
 
+   if (mq && mq->card && !mmc_card_inserted(mq->card))
+   return BLKPREP_KILL;
+
req->cmd_flags |= REQ_DONTPREP;
 
return BLKPREP_OK;
diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 46b6e84..ea3be5d 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -308,6 +308,7 @@ int mmc_add_card(struct mmc_card *card)
mmc_card_ddr_mode(card) ? "DDR " : "",
type, card->rca);
}
+   mmc_card_set_inserted(card);
 
 #ifdef CONFIG_DEBUG_FS
mmc_add_card_debugfs(card);
@@ -340,6 +341,7 @@ void mmc_remove_card(struct mmc_card *card)
pr_info("%s: card %04x removed\n",
mmc_hostname(card->host), card->rca);
}
+   card->state &= ~MMC_STATE_INSERTED;
device_del(&card->dev);
}
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index b7a8cb1..be4125a 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -209,6 +209,7 @@ struct mmc_card {
 #define MMC_STATE_HIGHSPEED_DDR (1<<4) /* card is in high speed mode */
 #define MMC_STATE_ULTRAHIGHSPEED (1<<5)/* card is in ultra 
high speed mode */
 #define MMC_CARD_SDXC  (1<<6)  /* card is SDXC */
+#define MMC_STATE_INSERTED (1<<7)  /* card present in the slot */
unsigned intquirks; /* card quirks */
 #define MMC_QUIRK_LENIENT_FN0  (1<<0)  /* allow SDIO FN0 writes 
outside of the VS CCCR range */
 #define MMC_QUIRK_BLKSZ_FOR_BYTE_MODE (1<<1)   /* use func->cur_blksize */
@@ -362,6 +363,7 @@ static inline void __maybe_unused remove_quirk(struct 
mmc_card *card, int data)
 #define mmc_card_sdio(c)   ((c)->type == MMC_TYPE_SDIO)
 
 #define mmc_card_present(c)((c)->state & MMC_STATE_PRESENT)
+#define mmc_card_inserted(c)   ((c)->state & MMC_STATE_INSERTED)
 #define mmc_card_readonly(c)   ((c)->state & MMC_STATE_READONLY)
 #define mmc_card_highspeed(c)  ((c)->state & MMC_STATE_HIGHSPEED)
 #define mmc_card_blockaddr(c)  ((c)->state & MMC_STATE_BLOCKADDR)
@@ -370,6 +372,7 @@ static inline void __maybe_unused remove_quirk(struct 
mmc_card *card, int data)
 #define mmc_card_ext_capacity(c) ((c)->state & MMC_CARD_SDXC)
 
 #define mmc_card_set_present(c)((c)->state |= MMC_STATE_PRESENT)
+#define mmc_card_set_inserted(c) ((c)->state |= MMC_STATE_INSERTED)
 #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
 #define mmc_card_set_highspeed(c) ((c)->state |= MMC_STATE_HIGHSPEED)
 #define mmc_card_set_blockaddr(c) ((c)->state |= MMC_STATE_BLOCKADDR)
-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] mmc: core: Use usleep_range for delays less than 20ms

2011-11-08 Thread Sujit Reddy Thumma

msleep of any delay less than 20ms (or 2 jiffies) would
give an unpredictable delay (most of the times as ~20ms).
Use usleep_range in mmc_delay() for delays less than two jiffies.

Signed-off-by: Sujit Reddy Thumma 
---
 drivers/mmc/core/core.h |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 14664f1..45a6a12 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -50,6 +50,8 @@ static inline void mmc_delay(unsigned int ms)
if (ms < 1000 / HZ) {
cond_resched();
mdelay(ms);
+   } else if (ms < jiffies_to_msecs(2)) {
+   usleep_range(ms * 1000, (ms + 1) * 1000);
} else {
msleep(ms);
}
-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] mmc: core: Use delayed work in clock gating framework

2011-10-24 Thread Sujit Reddy Thumma


On 10/24/2011 12:53 PM, Linus Walleij wrote:

On Wed, Oct 19, 2011 at 4:48 PM, Sujit Reddy Thumma
  wrote:


Current clock gating framework disables the MCI clock as soon as the
request is completed and enables it when a request arrives. This aggressive
clock gating framework when enabled cause following issues:

When there are back-to-back requests from the Queue layer, we unnecessarily
end up disabling and enabling the clocks between these requests since 8 MCLK
clock cycles is a very short duration compared to the time delay between
back to back requests reaching the MMC layer. This overhead can effect the
overall performance depending on how long the clock enable and disable
calls take which is platform dependent. For example on some platforms we
can have clock control not on the local processor, but on a different
subsystem and the time taken to perform the clock enable/disable can add
significant overhead.


OK I see the problem. At one point it was suggested to use the delayed
disable facilities in runtime PM for avoiding this, but it never materialized.


Fix this by delaying turning off the clocks by posting request on
delayed workqueue. Also cancel the unscheduled pending work, if any,
when there is access to card.


(...)

@@ -252,10 +252,11 @@ struct mmc_host {
int clk_requests;   /* internal reference counter */
unsigned intclk_delay;  /* number of MCI clk hold 
cycles */
boolclk_gated;  /* clock gated */
-   struct work_struct  clk_gate_work; /* delayed clock gate */
+   struct delayed_work clk_gate_work; /* delayed clock gate */
unsigned intclk_old;/* old clock value cache */
spinlock_t  clk_lock;   /* lock for clk fields */
struct mutexclk_gate_mutex; /* mutex for clock gating */
+#define MMC_CLK_GATE_DELAY 50  /* in milliseconds*/
  #endif


What's the rationale of having this in the middle of the struct
in the .h-file?

Move it inside the #ifdef in host.c instead, and also provide
some rationale about the choice of 50 ms.


With my testing on a MSM platform, round-trip delay of 10ms to turn off 
and on the clocks is seen. So I thought 50ms is a reasonable tradeoff. 
There is no specific calculation behind this delay. We just relied on 
the empirical data which again may vary with different platforms.


If you have any suggestions I would be happy to consider that. I am 
planning to increase this to 200ms, any comments on that?




Maybe we should even provide a sysfs or debugfs entry for
tweaking that value, as has been suggested in the past.
However that's no big deal for me...


Adding an entry in sysfs seems to be a good solution to tune the 
required delay. I will add this.




Do you have a patch to your msm_sdcc.c that enables the
gating to take effect as well? Does it solve the problems
pointed out by Russell when I tried the same type of patch
to mmci.c?
http://marc.info/?l=linux-mmc&m=129146545916794&w=2


Looking at the patch that you pointed out, I don't think it is required 
for msm_sdcc.c to enable clock gating. The current code inherently takes 
care of logic that is needed to ensure that MCLK is not turned off until 
any pending register write in the host driver is completed. You might 
want to have a look at set_ios function in 
https://www.codeaurora.org/gitweb/quic/la/?p=kernel/msm.git;a=blob;f=drivers/mmc/host/msm_sdcc.c;h=b5a08d2b58e0cd6d18a8f1041139b6866c0cdc09;hb=refs/heads/msm-3.0

Please let me know if I am still missing anything.

--
Thanks & Regards,
Sujit Reddy Thumma
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: core: Prevent too long response times for suspend

2011-10-19 Thread Sujit Reddy Thumma


On 10/17/2011 3:21 PM, Ulf Hansson wrote:


Why would there be pending requests while host is suspending? Is the
kernel framework not handling sync before going to suspend? However, the
mmc_blk_suspend() would be called before the host driver suspends (as all
the driver suspend routines are serialized) which means it stops block
layer to queue more I/O requests well before the host driver start
suspend. Does this sequence break in your case?


I have observed this issue for different cases (one case was logging to
eMMC). The idea is simply that we would like to be sure that we do not
wait "forever", no matter if the "upper layers" misbehaved in the
suspend sequence.

Agree. I was just curious to know why would there be a pending requests. 
If the issue is seen while logging to eMMC then there may be something 
wrong, probably, we may see issue while running mmc_test and initiating 
suspend. Your patch looks good to me though.


Reviewed-by: Sujit Reddy Thumma 



Your concern seems to be valid for SDIO case, but again the function
driver must be intelligent enough to return -EBUSY as it knows that it
had
posted a request to MMC.



For SDIO, should we really assume that function driver has implemented a
suspend function and moreover that it actually always behaves as we expect?

I hope so, but yes, in some cases that may be broken. I agree with you.


Br
Ulf Hansson


--
Thanks & Regards,
Sujit Reddy Thumma
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/1] mmc: core: Use delayed work in clock gating framework

2011-10-19 Thread Sujit Reddy Thumma

Current clock gating framework disables the MCI clock as soon as the
request is completed and enables it when a request arrives. This aggressive
clock gating framework when enabled cause following issues:

When there are back-to-back requests from the Queue layer, we unnecessarily
end up disabling and enabling the clocks between these requests since 8 MCLK
clock cycles is a very short duration compared to the time delay between
back to back requests reaching the MMC layer. This overhead can effect the
overall performance depending on how long the clock enable and disable
calls take which is platform dependent. For example on some platforms we
can have clock control not on the local processor, but on a different
subsystem and the time taken to perform the clock enable/disable can add
significant overhead.

Also if the host controller driver decides to disable the host clock too
when mmc_set_ios function is called with ios.clock=0, it adds additional
delay and it is highly possible that the next request had already arrived
and unnecessarily blocked in enabling the clocks. This is seen frequently
when the processor is executing at high speeds and in multi-core platforms
thus reduces the overall throughput compared to if clock gating is disabled.

Fix this by delaying turning off the clocks by posting request on
delayed workqueue. Also cancel the unscheduled pending work, if any,
when there is access to card.

Signed-off-by: Sujit Reddy Thumma 
---
 drivers/mmc/core/host.c  |   11 +++
 include/linux/mmc/host.h |3 ++-
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index ca2e4f5..b7b51da 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -113,7 +113,7 @@ static void mmc_host_clk_gate_delayed(struct mmc_host *host)
 static void mmc_host_clk_gate_work(struct work_struct *work)
 {
struct mmc_host *host = container_of(work, struct mmc_host,
- clk_gate_work);
+ clk_gate_work.work);
 
mmc_host_clk_gate_delayed(host);
 }
@@ -130,6 +130,8 @@ void mmc_host_clk_hold(struct mmc_host *host)
 {
unsigned long flags;
 
+   /* cancel any clock gating work scheduled by mmc_host_clk_release() */
+   cancel_delayed_work_sync(&host->clk_gate_work);
mutex_lock(&host->clk_gate_mutex);
spin_lock_irqsave(&host->clk_lock, flags);
if (host->clk_gated) {
@@ -179,7 +181,8 @@ void mmc_host_clk_release(struct mmc_host *host)
host->clk_requests--;
if (mmc_host_may_gate_card(host->card) &&
!host->clk_requests)
-   queue_work(system_nrt_wq, &host->clk_gate_work);
+   queue_delayed_work(system_nrt_wq, &host->clk_gate_work,
+   msecs_to_jiffies(MMC_CLK_GATE_DELAY));
spin_unlock_irqrestore(&host->clk_lock, flags);
 }
 
@@ -213,7 +216,7 @@ static inline void mmc_host_clk_init(struct mmc_host *host)
/* Hold MCI clock for 8 cycles by default */
host->clk_delay = 8;
host->clk_gated = false;
-   INIT_WORK(&host->clk_gate_work, mmc_host_clk_gate_work);
+   INIT_DELAYED_WORK(&host->clk_gate_work, mmc_host_clk_gate_work);
spin_lock_init(&host->clk_lock);
mutex_init(&host->clk_gate_mutex);
 }
@@ -228,7 +231,7 @@ static inline void mmc_host_clk_exit(struct mmc_host *host)
 * Wait for any outstanding gate and then make sure we're
 * ungated before exiting.
 */
-   if (cancel_work_sync(&host->clk_gate_work))
+   if (cancel_delayed_work_sync(&host->clk_gate_work))
mmc_host_clk_gate_delayed(host);
if (host->clk_gated)
mmc_host_clk_hold(host);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 16e9338..8d6ba4c 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -252,10 +252,11 @@ struct mmc_host {
int clk_requests;   /* internal reference counter */
unsigned intclk_delay;  /* number of MCI clk hold 
cycles */
boolclk_gated;  /* clock gated */
-   struct work_struct  clk_gate_work; /* delayed clock gate */
+   struct delayed_work clk_gate_work; /* delayed clock gate */
unsigned intclk_old;/* old clock value cache */
spinlock_t  clk_lock;   /* lock for clk fields */
struct mutexclk_gate_mutex; /* mutex for clock gating */
+#define MMC_CLK_GATE_DELAY 50  /* in milliseconds*/
 #endif
 
/* host specific block data */
-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

--
To unsubscribe from

Re: [PATCH] mmc: core: Prevent too long response times for suspend

2011-10-14 Thread Sujit Reddy Thumma

> While trying to suspend the mmc host there could still be
> ongoing requests that we need to wait for. At the same time
> a device driver must respond to a suspend request rather quickly.
>
> Instead of potentially wait "forever" by claiming the host we now
> "try" to claim the host instead. If it fails, -EBUSY is returned.
>
> Signed-off-by: Ulf Hansson 
> ---
>  drivers/mmc/core/core.c |   42 +++---
>  1 files changed, 27 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 61d7730..b900932 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2121,21 +2121,33 @@ int mmc_suspend_host(struct mmc_host *host)
>
>   mmc_bus_get(host);
>   if (host->bus_ops && !host->bus_dead) {
> - if (host->bus_ops->suspend)
> - err = host->bus_ops->suspend(host);
> - if (err == -ENOSYS || !host->bus_ops->resume) {
> - /*
> -  * We simply "remove" the card in this case.
> -  * It will be redetected on resume.
> -  */
> - if (host->bus_ops->remove)
> - host->bus_ops->remove(host);
> - mmc_claim_host(host);
> - mmc_detach_bus(host);
> - mmc_power_off(host);
> - mmc_release_host(host);
> - host->pm_flags = 0;
> - err = 0;
> +
> + /*
> +  * A long response time is not acceptable for device drivers
> +  * when doing suspend. Prevent mmc_claim_host in the suspend
> +  * sequence, to potentially wait "forever" by trying to
> +  * pre-claim the host.
> +  */

Why would there be pending requests while host is suspending? Is the
kernel framework not handling sync before going to suspend? However, the
mmc_blk_suspend() would be called before the host driver suspends (as all
the driver suspend routines are serialized) which means it stops block
layer to queue more I/O requests well before the host driver start
suspend. Does this sequence break in your case?

Your concern seems to be valid for SDIO case, but again the function
driver must be intelligent enough to return -EBUSY as it knows that it had
posted a request to MMC.

> + if (mmc_try_claim_host(host)) {
> + if (host->bus_ops->suspend)
> + err = host->bus_ops->suspend(host);
> + if (err == -ENOSYS || !host->bus_ops->resume) {
> + /*
> +  * We simply "remove" the card in this case.
> +  * It will be redetected on resume.
> +  */
> + if (host->bus_ops->remove)
> + host->bus_ops->remove(host);
> + mmc_claim_host(host);
> + mmc_detach_bus(host);
> + mmc_power_off(host);
> + mmc_release_host(host);
> + host->pm_flags = 0;
> + err = 0;
> + }
> + mmc_do_release_host(host);
> + } else {
> + err = -EBUSY;
>   }
>   }
>   mmc_bus_put(host);
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

--
Thanks,
Sujit

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] mmc: core: Kill block requests if card is removed

2011-10-13 Thread Sujit Reddy Thumma


On 10/12/2011 4:04 PM, Pavan Kondeti wrote:

Hello Sujit,

On 10/12/2011 1:06 PM, Sujit Reddy Thumma wrote:

Kill block requests when the host knows that the card is
removed from the slot and is sure that it can no longer
accept any requests.

Kill this silently so that the block layer don't output
error messages unnecessarily.

Signed-off-by: Sujit Reddy Thumma





diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 0ea4a06..7cdbc14 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -196,6 +196,7 @@ struct mmc_card {
  #define MMC_STATE_HIGHSPEED_DDR (1<<4)  /* card is in high speed mode 
*/
  #define MMC_STATE_ULTRAHIGHSPEED (1<<5) /* card is in ultra high 
speed mode */
  #define MMC_CARD_SDXC (1<<6)/* card is SDXC */
+#define MMC_STATE_INSERTED  (1<<7)  /* card present in the slot */
unsigned intquirks; /* card quirks */
  #define MMC_QUIRK_LENIENT_FN0 (1<<0)/* allow SDIO FN0 writes 
outside of the VS CCCR range */
  #define MMC_QUIRK_BLKSZ_FOR_BYTE_MODE (1<<1)/* use func->cur_blksize */
@@ -344,6 +345,7 @@ static inline void __maybe_unused remove_quirk(struct 
mmc_card *card, int data)
  #define mmc_card_sdio(c)  ((c)->type == MMC_TYPE_SDIO)

  #define mmc_card_present(c)   ((c)->state&  MMC_STATE_PRESENT)
+#define mmc_card_inserted(c) ((c)->state&  MMC_STATE_INSERTED)
  #define mmc_card_readonly(c)  ((c)->state&  MMC_STATE_READONLY)
  #define mmc_card_highspeed(c) ((c)->state&  MMC_STATE_HIGHSPEED)
  #define mmc_card_blockaddr(c) ((c)->state&  MMC_STATE_BLOCKADDR)
@@ -352,6 +354,7 @@ static inline void __maybe_unused remove_quirk(struct 
mmc_card *card, int data)
  #define mmc_card_ext_capacity(c) ((c)->state&  MMC_CARD_SDXC)

  #define mmc_card_set_present(c)   ((c)->state |= MMC_STATE_PRESENT)
+#define mmc_card_set_inserted(c) ((c)->state |= MMC_STATE_INSERTED)
  #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
  #define mmc_card_set_highspeed(c) ((c)->state |= MMC_STATE_HIGHSPEED)
  #define mmc_card_set_blockaddr(c) ((c)->state |= MMC_STATE_BLOCKADDR)


Why do we need another flag to indicate card's presence? can not we use
MMC_STATE_PRESENT flag? This flag is set in mmc_add_card(). But not
cleared any where...

MMC_STATE_PRESENT signifies the card presence in sysfs i.e. after 
device_add() is called we can be sure that kobject for card device is 
created. This will be cleared automatically when we do 
mmc_release_card() after card removal. But before this when doing 
device_del() is called the FS layer flush/sync the dirty data onto card. 
This creates unnecessary noise in dmesg log when the card is removed 
from the slot (without unmounting).

Thanks,
Pavan



Thanks,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC/PATCH] mmc: core: Kill block requests if card is removed

2011-10-12 Thread Sujit Reddy Thumma

Kill block requests when the host knows that the card is
removed from the slot and is sure that it can no longer
accept any requests.

Kill this silently so that the block layer don't output
error messages unnecessarily.

Signed-off-by: Sujit Reddy Thumma 

diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index fed290e..e1961db 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -29,6 +29,8 @@
  */
 static int mmc_prep_request(struct request_queue *q, struct request *req)
 {
+   struct mmc_queue *mq = q->queuedata;
+
/*
 * We only like normal block requests and discards.
 */
@@ -37,6 +39,9 @@ static int mmc_prep_request(struct request_queue *q, struct 
request *req)
return BLKPREP_KILL;
}
 
+   if (mq && mq->card && !mmc_card_inserted(mq->card))
+   return BLKPREP_KILL;
+
req->cmd_flags |= REQ_DONTPREP;
 
return BLKPREP_OK;
diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 46b6e84..ea3be5d 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -308,6 +308,7 @@ int mmc_add_card(struct mmc_card *card)
mmc_card_ddr_mode(card) ? "DDR " : "",
type, card->rca);
}
+   mmc_card_set_inserted(card);
 
 #ifdef CONFIG_DEBUG_FS
mmc_add_card_debugfs(card);
@@ -340,6 +341,7 @@ void mmc_remove_card(struct mmc_card *card)
pr_info("%s: card %04x removed\n",
mmc_hostname(card->host), card->rca);
}
+   card->state &= ~MMC_STATE_INSERTED;
device_del(&card->dev);
}
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 0ea4a06..7cdbc14 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -196,6 +196,7 @@ struct mmc_card {
 #define MMC_STATE_HIGHSPEED_DDR (1<<4) /* card is in high speed mode */
 #define MMC_STATE_ULTRAHIGHSPEED (1<<5)/* card is in ultra 
high speed mode */
 #define MMC_CARD_SDXC  (1<<6)  /* card is SDXC */
+#define MMC_STATE_INSERTED  (1<<7)  /* card present in the slot */
unsigned intquirks; /* card quirks */
 #define MMC_QUIRK_LENIENT_FN0  (1<<0)  /* allow SDIO FN0 writes 
outside of the VS CCCR range */
 #define MMC_QUIRK_BLKSZ_FOR_BYTE_MODE (1<<1)   /* use func->cur_blksize */
@@ -344,6 +345,7 @@ static inline void __maybe_unused remove_quirk(struct 
mmc_card *card, int data)
 #define mmc_card_sdio(c)   ((c)->type == MMC_TYPE_SDIO)
 
 #define mmc_card_present(c)((c)->state & MMC_STATE_PRESENT)
+#define mmc_card_inserted(c) ((c)->state & MMC_STATE_INSERTED)
 #define mmc_card_readonly(c)   ((c)->state & MMC_STATE_READONLY)
 #define mmc_card_highspeed(c)  ((c)->state & MMC_STATE_HIGHSPEED)
 #define mmc_card_blockaddr(c)  ((c)->state & MMC_STATE_BLOCKADDR)
@@ -352,6 +354,7 @@ static inline void __maybe_unused remove_quirk(struct 
mmc_card *card, int data)
 #define mmc_card_ext_capacity(c) ((c)->state & MMC_CARD_SDXC)
 
 #define mmc_card_set_present(c)((c)->state |= MMC_STATE_PRESENT)
+#define mmc_card_set_inserted(c) ((c)->state |= MMC_STATE_INSERTED)
 #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
 #define mmc_card_set_highspeed(c) ((c)->state |= MMC_STATE_HIGHSPEED)
 #define mmc_card_set_blockaddr(c) ((c)->state |= MMC_STATE_BLOCKADDR)
-- 
1.7.3.3

--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC] mmc: ignore asynchronous calls on dead buses

2011-06-15 Thread Sujit Reddy Thumma


On 6/15/2011 6:58 PM, Guennadi Liakhovetski wrote:

MMC host drivers have three main asynchronous event types, that they
report to the MMC core: request completions, SDIO interrupts and card
hotplug events. Avoid processing these calls during driver removal.

Signed-off-by: Guennadi Liakhovetski
---

This is my attempt to answer my own question:

http://article.gmane.org/gmane.linux.kernel.mmc/8280

All these races are very unlikely, but can be triggered artificially by
inserting a delay in host drivers after mmc_remove_host().

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 68091dd..c11e47b 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -95,6 +95,9 @@ void mmc_request_done(struct mmc_host *host, struct 
mmc_request *mrq)
struct mmc_command *cmd = mrq->cmd;
int err = cmd->error;

+   if (host->bus_dead)
+   return;
+
host->bus_dead is set when there are no interesting cards left on the 
bus. It doesn't mean host driver being removed. Probably, you should use 
host->removed instead.

if (err&&  cmd->retries&&  mmc_host_is_spi(host)) {
if (cmd->resp[0]&  R1_SPI_ILLEGAL_COMMAND)
cmd->retries = 0;
@@ -1162,7 +1165,8 @@ void mmc_detect_change(struct mmc_host *host, unsigned 
long delay)
spin_unlock_irqrestore(&host->lock, flags);
  #endif

-   mmc_schedule_delayed_work(&host->detect, delay);
+   if (!host->bus_dead)
+   mmc_schedule_delayed_work(&host->detect, delay);
  }

  EXPORT_SYMBOL(mmc_detect_change);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 1ee4424..1a1f2a4 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -311,6 +311,9 @@ extern void mmc_request_done(struct mmc_host *, struct 
mmc_request *);

  static inline void mmc_signal_sdio_irq(struct mmc_host *host)
  {
+   if (host->bus_dead)
+   return;
+
host->ops->enable_sdio_irq(host, 0);
wake_up_process(host->sdio_irq_thread);
  }
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



Thanks
Sujit

--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

54 matches

Mail list logo