from:"Sonny Rao"

Re: [PATCH V2 3/3] mmc: dw_mmc: Dont cut off vqmmc and vmmc

2014-08-28 Thread Sonny Rao

On Thu, Aug 28, 2014 at 8:50 AM, Doug Anderson diand...@google.com wrote:
 Ulf,

 On Thu, Aug 28, 2014 at 12:25 AM, Ulf Hansson ulf.hans...@linaro.org wrote:
 On 27 August 2014 17:52, Doug Anderson diand...@google.com wrote:
 Ulf,

 On Wed, Aug 27, 2014 at 4:17 AM, Ulf Hansson ulf.hans...@linaro.org wrote:
 Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be
 called to check the card detect line, but with vmmc and vqmmc off.  It
 will be unable to return a sensible value without actually turning on
 vmmc and vqmmc.

 Currently MMC_CAP_NEEDS_POLL mean the mmc rescan work will reschedule
 itself with HZ interval. This to check for card insert/removal.

 Now, mmc_rescan() will, if present, invoke host's -get_cd() callback
 to check whether it's worth to continue initialization sequence or if
 it should re-schedule itself again.

 If your host driver can distinguish whether a card is inserted, which
 in this case are when vccq is turned off, your -get_cd() callback need
 to return 1. That will allow mmc_rescan() to continue it's
 initialization sequence and do mmc_power_up().

 As per my other email, we can't tell whether a card is inserted when
 vqmmc is off.

 I understand.

 The solution I proposed above, is:

 1) Enable MMC_CAP_NEEDS_POLL.
 2) Make -get_cd() return 1, when you can't distinguish if the card is 
 inserted.

 That will solve this issue and without messing up the mmc core.

 Ah, I see!  So every 1 second, we'll do the following:

 1. Call get_cd(), which returns 1.

 2. MMC core will power everything up (turn on all regulators) for MMC.

 3. We'll start to initialize a card.

 4. We'll notice that, oops, there's no card there.

 5. MMC core will shut things down again.


 That seems awfully expensive to do every second when the card detect
 line actually does work (as long as you keep power lines).  Is the
 patch to separate out the concepts of power off because no card is
 inserted and power off because we're power cycling the card really
 bad enough to warrant forcing us to use the above?

 I'm not an EE, but cycling regulators on and off every second doesn't
 seem super ideal, but maybe they're designed for it?


 Personally, I'd be tempted to just leave the power on all the time and
 if a card somehow needs to be power cycled (because UHS negotiation
 failed?) then that card just isn't supported.  This could be done in
 the dts by saying that the regulator is always on and no need to
 pollute any source files.

Yeah, power cycling the regulators constantly doesn't seem like a
great idea, we can ask the EEs what they think.

This scheme of leaving them on all the time would prevent us from
being able to actually power cycle the card, which I think is required
by the spec in case UHS negotiation fails.




 -Doug
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc 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 3/3] mmc: dw_mmc: Dont cut off vqmmc and vmmc

2014-08-22 Thread Sonny Rao

On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson ulf.hans...@linaro.org wrote:
 On 22 August 2014 15:47, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote:
 Exynos 5250 and 5420 based boards uses built-in CD# line for card
 detection.But unfortunately CD# line is on the same voltage rails
 as of I/O voltage rails. When we cut off vqmmc,the consequent card
 detection will break in these boards.

 I am not sure I follow here.

 Is the card detect mechanism handled internally by the dw_mmc controller?

Yes


 I thought HW engineers long time ago realized that this should be done
 separately on a GPIO line to be able to save power while waiting for a
 card to be inserted. But that's not case then?

At least in my limited experience, this seems to be common among SoC
vendors who are using dw_mmc, as we've seen this elsewhere as well and
after seeing it here we know that we need to ignore the CD pin that's
routed to dw_mmc and use a separately powered GPIO on the board, but
still there are probably many SoCs/boards which are doing it this way.


 These hosts (obviously) need to keep vqmmc (and thus vmmc) on all the
 time, even when the mmc core tells them to power off. However, one
 problem is that these cards won't properly handle mmc_power_cycle().
 That's needed to handle error cases when trying to switch voltages
 (see 0797e5f mmc:core: Fixup signal voltage switch).

 This patch adds a new MMC_POWER_OFF_HARD mode when it's doing a power
 cycle.  This mode differs from the normal MMC_POWER_OFF mode in that
 the mmc core will promise to power the slot back on before it expects
 the host to detect card insertion or removal.

This patch is based off of one that Doug wrote (sent privately to
Yuvaraj) which just modifies the MMC core, and should be split into
two patches.
One that modifies the mmc core and one that implements this in dw_mmc.


 Also if we let alone the vqmmc turned on when vmmc turned off, the
 card could have half way powered and this can damage the card. So
 this patch adds a check so that, if the board used the built-in
 card detection mechanism i.e through CDETECT, it will not turned
 down vqmmc and vmmc both.

 Why does vmmc needs to be enabled when there are no card inserted?
 That can't be right?

I think this is because we don't want to send power to a card over the
I/O pins but not the vcc pin, even for a little while. We also asked
some SD card manufacturers about whether it was okay to leave vqmmc on
and they recommended not doing this, because there's a risk of damage
to the card.

So, in this (broken) environment, we basically just keep both vmmc and
vqmmc on all the time unless mmc core is asking the driver to power
cycle the card through MMC_POWER_OFF and MMC_POWER_ON modes.

Does that help?


 Kind regards
 Uffe


 Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
 Signed-off-by: Doug Anderson diand...@chromium.org
 ---
 changes from v1:
 1.added a new MMC_POWER_OFF_HARD mode as per Doug Anderson's 
 suggestion.
 2.added dw_mci_exynos_post_init() to perform the host specific post
   initialisation.
 3.added a new flag MMC_CAP2_CD_NEEDS_POWER for host-caps2.

  drivers/mmc/core/core.c  |   16 ++--
  drivers/mmc/core/debugfs.c   |3 +++
  drivers/mmc/host/dw_mmc-exynos.c |   12 
  drivers/mmc/host/dw_mmc.c|   25 +
  drivers/mmc/host/dw_mmc.h|2 ++
  include/linux/mmc/host.h |2 ++
  6 files changed, 58 insertions(+), 2 deletions(-)

 diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
 index 68f5f4b..79ced36 100644
 --- a/drivers/mmc/core/core.c
 +++ b/drivers/mmc/core/core.c
 @@ -1564,9 +1564,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
 mmc_host_clk_release(host);
  }

 -void mmc_power_off(struct mmc_host *host)
 +void _mmc_power_off(struct mmc_host *host, unsigned char power_mode)
  {
 -   if (host-ios.power_mode == MMC_POWER_OFF)
 +   if (host-ios.power_mode == power_mode)
 return;

 mmc_host_clk_hold(host);
 @@ -1579,6 +1579,7 @@ void mmc_power_off(struct mmc_host *host)
 host-ios.chip_select = MMC_CS_DONTCARE;
 }
 host-ios.power_mode = MMC_POWER_OFF;
 +   host-ios.power_mode = power_mode;
 host-ios.bus_width = MMC_BUS_WIDTH_1;
 host-ios.timing = MMC_TIMING_LEGACY;
 mmc_set_ios(host);
 @@ -1593,9 +1594,20 @@ void mmc_power_off(struct mmc_host *host)
 mmc_host_clk_release(host);
  }

 +void mmc_power_off(struct mmc_host *host)
 +{
 +   _mmc_power_off(host, MMC_POWER_OFF);
 +}
 +
  void mmc_power_cycle(struct mmc_host *host, u32 ocr)
  {
 mmc_power_off(host);
 +   /* If host normally ignores MMC_POWER_OFF, tell it to pay attention 
 */
 +   if (host-caps2  MMC_CAP2_CD_NEEDS_POWER)
 +   _mmc_power_off(host, MMC_POWER_OFF_HARD);
 +   else
 +   _mmc_power_off(host, MMC_POWER_OFF);
 +

[PATCH] mmc: dw_mmc: change to use recommended reset procedure

2014-08-04 Thread Sonny Rao

This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook.

Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
Acked-by: Seungwon Jeon tgih@samsung.com
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
[sonnyrao: fix compile for !CONFIG_MMC_DW_IDMAC case]
---
 drivers/mmc/host/dw_mmc.c | 87 ++-
 drivers/mmc/host/dw_mmc.h |  5 +++
 2 files changed, 69 insertions(+), 23 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 1ac227c..39cf54f 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -111,8 +111,7 @@ static const u8 tuning_blk_pattern_8bit[] = {
0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
 };
 
-static inline bool dw_mci_fifo_reset(struct dw_mci *host);
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host);
+static bool dw_mci_reset(struct dw_mci *host);
 
 #if defined(CONFIG_DEBUG_FS)
 static int dw_mci_req_show(struct seq_file *s, void *v)
@@ -1235,7 +1234,7 @@ static int dw_mci_data_complete(struct dw_mci *host, 
struct mmc_data *data)
 * After an error, there may be data lingering
 * in the FIFO
 */
-   dw_mci_fifo_reset(host);
+   dw_mci_reset(host);
} else {
data-bytes_xfered = data-blocks * data-blksz;
data-error = 0;
@@ -1352,7 +1351,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
 
/* CMD error in data command */
if (mrq-cmd-error  mrq-data)
-   dw_mci_fifo_reset(host);
+   dw_mci_reset(host);
 
host-cmd = NULL;
host-data = NULL;
@@ -1963,14 +1962,8 @@ static void dw_mci_work_routine_card(struct work_struct 
*work)
}
 
/* Power down slot */
-   if (present == 0) {
-   /* Clear down the FIFO */
-   dw_mci_fifo_reset(host);
-#ifdef CONFIG_MMC_DW_IDMAC
-   dw_mci_idmac_reset(host);
-#endif
-
-   }
+   if (present == 0)
+   dw_mci_reset(host);
 
spin_unlock_bh(host-lock);
 
@@ -2208,8 +2201,11 @@ static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 
reset)
return false;
 }
 
-static inline bool dw_mci_fifo_reset(struct dw_mci *host)
+static bool dw_mci_reset(struct dw_mci *host)
 {
+   u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
+   bool ret = false;
+
/*
 * Reseting generates a block interrupt, hence setting
 * the scatter-gather pointer to NULL.
@@ -2219,15 +2215,60 @@ static inline bool dw_mci_fifo_reset(struct dw_mci 
*host)
host-sg = NULL;
}
 
-   return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
-}
+   if (host-use_dma)
+   flags |= SDMMC_CTRL_DMA_RESET;
 
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
-{
-   return dw_mci_ctrl_reset(host,
-SDMMC_CTRL_FIFO_RESET |
-SDMMC_CTRL_RESET |
-SDMMC_CTRL_DMA_RESET);
+   if (dw_mci_ctrl_reset(host, flags)) {
+   /*
+* In all cases we clear the RAWINTS register to clear any
+* interrupts.
+*/
+   mci_writel(host, RINTSTS, 0x);
+
+   /* if using dma we wait for dma_req to clear */
+   if (host-use_dma) {
+   unsigned long timeout = jiffies + msecs_to_jiffies(500);
+   u32 status;
+   do {
+   status = mci_readl(host, STATUS);
+   if (!(status  SDMMC_STATUS_DMA_REQ))
+   break;
+   cpu_relax();
+   } while (time_before(jiffies, timeout));
+
+   if (status  SDMMC_STATUS_DMA_REQ) {
+   dev_err(host-dev,
+   %s: Timeout waiting for dma_req to 
+   clear during reset\n, __func__);
+   goto ciu_out;
+   }
+
+   /* when using DMA next we reset the fifo again */
+   if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET))
+   goto ciu_out;
+   }
+   } else {
+   /* if the controller reset bit did clear, then set clock regs */
+   if (!(mci_readl(host, CTRL)  SDMMC_CTRL_RESET)) {
+   dev_err(host

[PATCHv6] mmc: dw_mmc: change to use recommended reset procedure

2014-07-14 Thread Sonny Rao

This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook.

Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
---
v2: Add Generic DMA support
per the documentation, move interrupt clear before wait
make the test for DMA host-use_dma rather than host-using_dma
add proper return values (although it appears no caller checks)
v3: rename fifo reset function, and change callers
use this combined reset function in dw_mci_resume()
just one caller left (probe), so get rid of dw_mci_ctrl_all_reset()
use DMA reset bit for all systems which use DMA
remove extra IDMAC reset in dw_mci_work_routine_card()
do CIU clock update in error path, if CIU reset cleared
v4: remove comment about FIFO reset in dw_mci_work_routine_card()
move down error message when control reset clears but others don't
 and clarify the error stating that we will still update clocks
make flags for all reset bits a macro
v5: don't use dw_mci_reset() in dw_mci_resume() and instead match what
is done in dw_mci_probe() and don't force inline dw_mci_resume()
v6: add newlines to dev_err() messages
---
 drivers/mmc/host/dw_mmc.c | 86 ++-
 drivers/mmc/host/dw_mmc.h |  5 +++
 2 files changed, 68 insertions(+), 23 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 55cd110..0c0aecb 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -111,8 +111,7 @@ static const u8 tuning_blk_pattern_8bit[] = {
0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
 };
 
-static inline bool dw_mci_fifo_reset(struct dw_mci *host);
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host);
+static bool dw_mci_reset(struct dw_mci *host);
 
 #if defined(CONFIG_DEBUG_FS)
 static int dw_mci_req_show(struct seq_file *s, void *v)
@@ -1254,7 +1253,7 @@ static int dw_mci_data_complete(struct dw_mci *host, 
struct mmc_data *data)
 * After an error, there may be data lingering
 * in the FIFO
 */
-   dw_mci_fifo_reset(host);
+   dw_mci_reset(host);
} else {
data-bytes_xfered = data-blocks * data-blksz;
data-error = 0;
@@ -1371,7 +1370,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
 
/* CMD error in data command */
if (mrq-cmd-error  mrq-data)
-   dw_mci_fifo_reset(host);
+   dw_mci_reset(host);
 
host-cmd = NULL;
host-data = NULL;
@@ -1982,14 +1981,8 @@ static void dw_mci_work_routine_card(struct work_struct 
*work)
}
 
/* Power down slot */
-   if (present == 0) {
-   /* Clear down the FIFO */
-   dw_mci_fifo_reset(host);
-#ifdef CONFIG_MMC_DW_IDMAC
-   dw_mci_idmac_reset(host);
-#endif
-
-   }
+   if (present == 0)
+   dw_mci_reset(host);
 
spin_unlock_bh(host-lock);
 
@@ -2323,8 +2316,11 @@ static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 
reset)
return false;
 }
 
-static inline bool dw_mci_fifo_reset(struct dw_mci *host)
+static bool dw_mci_reset(struct dw_mci *host)
 {
+   u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
+   bool ret = false;
+
/*
 * Reseting generates a block interrupt, hence setting
 * the scatter-gather pointer to NULL.
@@ -2334,15 +2330,59 @@ static inline bool dw_mci_fifo_reset(struct dw_mci 
*host)
host-sg = NULL;
}
 
-   return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
-}
+   if (host-use_dma)
+   flags |= SDMMC_CTRL_DMA_RESET;
 
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
-{
-   return dw_mci_ctrl_reset(host,
-SDMMC_CTRL_FIFO_RESET |
-SDMMC_CTRL_RESET |
-SDMMC_CTRL_DMA_RESET);
+   if (dw_mci_ctrl_reset(host, flags)) {
+   /*
+* In all cases we clear the RAWINTS register to clear any
+* interrupts.
+*/
+   mci_writel(host, RINTSTS, 0x);
+
+   /* if using dma we wait for dma_req to clear */
+   if (host-use_dma) {
+   unsigned long timeout = jiffies + msecs_to_jiffies(500);
+   u32 status;
+   do {
+   status = mci_readl(host, STATUS);
+   if (!(status  SDMMC_STATUS_DMA_REQ))
+   break

Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure

2014-07-11 Thread Sonny Rao

On Fri, Jul 11, 2014 at 3:20 AM, Seungwon Jeon tgih@samsung.com wrote:
 On Fri, July 11, 2014, Sonny Rao wrote:
 On Thu, Jul 10, 2014 at 5:28 AM, Seungwon Jeon tgih@samsung.com wrote:
  Hi Sonny,
 
  I have missed this patch.
 
  You finally choose to take extra interrupt handling.
  If it is not harm, it's fine.

 Hi, thanks for coming back to it.  Based on my tracing, the interrupt
 seems to be okay and is just ignored.

  -static inline bool dw_mci_fifo_reset(struct dw_mci *host)
  +static inline bool dw_mci_reset(struct dw_mci *host)
 inline is no needed anymore.

   {
  + u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
  + bool ret = false;
  +
/*
 * Reseting generates a block interrupt, hence setting
 * the scatter-gather pointer to NULL.
  @@ -2334,15 +2330,59 @@ static inline bool dw_mci_fifo_reset(struct 
  dw_mci *host)
host-sg = NULL;
}
 
  - return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
  -}
  + if (host-use_dma)
  + flags |= SDMMC_CTRL_DMA_RESET;
 
  -static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
  -{
  - return dw_mci_ctrl_reset(host,
  -  SDMMC_CTRL_FIFO_RESET |
  -  SDMMC_CTRL_RESET |
  -  SDMMC_CTRL_DMA_RESET);
  + if (dw_mci_ctrl_reset(host, flags)) {
  + /*
  +  * In all cases we clear the RAWINTS register to clear any
  +  * interrupts.
  +  */
  + mci_writel(host, RINTSTS, 0x);
  +
  + /* if using dma we wait for dma_req to clear */
  + if (host-use_dma) {
  + unsigned long timeout = jiffies + 
  msecs_to_jiffies(500);
  + u32 status;
  + do {
  + status = mci_readl(host, STATUS);
  + if (!(status  SDMMC_STATUS_DMA_REQ))
  + break;
  + cpu_relax();
  + } while (time_before(jiffies, timeout));
  +
  + if (status  SDMMC_STATUS_DMA_REQ) {
  + dev_err(host-dev,
  + %s: Timeout waiting for dma_req to 
  
  + clear during reset, __func__);
  + goto ciu_out;
  + }
  +
  + /* when using DMA next we reset the fifo again */
  + if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET))
  + goto ciu_out;
  + }
  + } else {
  + /* if the controller reset bit did clear, then set clock 
  regs */
  + if (!(mci_readl(host, CTRL)  SDMMC_CTRL_RESET)) {
  + dev_err(host-dev, %s: fifo/dma reset bits didn't 
  + clear but ciu was reset, doing clock 
  update.,
  + __func__);
  + goto ciu_out;
  + }
  + }
  +
  + if (IS_ENABLED(CONFIG_MMC_DW_IDMAC))
  + /* It is also recommended that we reset and reprogram idmac 
  */
  + dw_mci_idmac_reset(host);
  +
  + ret = true;
  +
  +ciu_out:
  + /* After a CTRL reset we need to have CIU set clock registers  */
  + mci_send_cmd(host-cur_slot, SDMMC_CMD_UPD_CLK, 0);
  +
  + return ret;
   }
 
   #ifdef CONFIG_OF
  @@ -2555,7 +2595,7 @@ int dw_mci_probe(struct dw_mci *host)
}
 
/* Reset all blocks */
  - if (!dw_mci_ctrl_all_reset(host))
  + if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS))
return -ENODEV;
 
host-dma_ops = host-pdata-dma_ops;
  @@ -2744,7 +2784,7 @@ int dw_mci_resume(struct dw_mci *host)
}
}
 
  - if (!dw_mci_ctrl_all_reset(host)) {
  + if (!dw_mci_reset(host)) {
  Do you have any reason to use dw_mci_reset() unlike doing on probing?

 I really wanted to use dw_mci_reset() everwhere, including probe,
 because that would be simplest, where there is just one reset function
 always, but the host structure is not completely set up at probe time,
 so the code in dw_mci_reset() where we try to send a command to update
 clock fails, so that's why I had to just do a reset.

 Yes, if we can keep one interface, it would be good.
 But can you check below?
 Did you see the kernel panic on probing with host-cur_slot is NULL?

Yes, I think that was the one.

 If right, resume seems not different from probe in case of removable type.
 And dw_mci_idmac_reset() is redundant when dw_mci_reset() is used at resume.

 I think dw_mci_ctrl_reset() can be also used at resume like at probe for 
 simple way.
 For safety's sake, host-cur_slot should be guaranteed it's not NULL.

Ok, I didn't try on removable, but I can change it to match probe,
thanks for looking at this.


 Thanks

[PATCHv5] mmc: dw_mmc: change to use recommended reset procedure

2014-07-11 Thread Sonny Rao

This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook.

Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
---
v2: Add Generic DMA support
per the documentation, move interrupt clear before wait
make the test for DMA host-use_dma rather than host-using_dma
add proper return values (although it appears no caller checks)
v3: rename fifo reset function, and change callers
use this combined reset function in dw_mci_resume()
just one caller left (probe), so get rid of dw_mci_ctrl_all_reset()
use DMA reset bit for all systems which use DMA
remove extra IDMAC reset in dw_mci_work_routine_card()
do CIU clock update in error path, if CIU reset cleared
v4: remove comment about FIFO reset in dw_mci_work_routine_card()
move down error message when control reset clears but others don't
 and clarify the error stating that we will still update clocks
make flags for all reset bits a macro
v5: don't use dw_mci_reset() in dw_mci_resume() and instead match what
is done in dw_mci_probe() and don't force inline dw_mci_resume()
---
 drivers/mmc/host/dw_mmc.c | 86 ++-
 drivers/mmc/host/dw_mmc.h |  5 +++
 2 files changed, 68 insertions(+), 23 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 55cd110..db25494 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -111,8 +111,7 @@ static const u8 tuning_blk_pattern_8bit[] = {
0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
 };
 
-static inline bool dw_mci_fifo_reset(struct dw_mci *host);
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host);
+static bool dw_mci_reset(struct dw_mci *host);
 
 #if defined(CONFIG_DEBUG_FS)
 static int dw_mci_req_show(struct seq_file *s, void *v)
@@ -1254,7 +1253,7 @@ static int dw_mci_data_complete(struct dw_mci *host, 
struct mmc_data *data)
 * After an error, there may be data lingering
 * in the FIFO
 */
-   dw_mci_fifo_reset(host);
+   dw_mci_reset(host);
} else {
data-bytes_xfered = data-blocks * data-blksz;
data-error = 0;
@@ -1371,7 +1370,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
 
/* CMD error in data command */
if (mrq-cmd-error  mrq-data)
-   dw_mci_fifo_reset(host);
+   dw_mci_reset(host);
 
host-cmd = NULL;
host-data = NULL;
@@ -1982,14 +1981,8 @@ static void dw_mci_work_routine_card(struct work_struct 
*work)
}
 
/* Power down slot */
-   if (present == 0) {
-   /* Clear down the FIFO */
-   dw_mci_fifo_reset(host);
-#ifdef CONFIG_MMC_DW_IDMAC
-   dw_mci_idmac_reset(host);
-#endif
-
-   }
+   if (present == 0)
+   dw_mci_reset(host);
 
spin_unlock_bh(host-lock);
 
@@ -2323,8 +2316,11 @@ static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 
reset)
return false;
 }
 
-static inline bool dw_mci_fifo_reset(struct dw_mci *host)
+static bool dw_mci_reset(struct dw_mci *host)
 {
+   u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
+   bool ret = false;
+
/*
 * Reseting generates a block interrupt, hence setting
 * the scatter-gather pointer to NULL.
@@ -2334,15 +2330,59 @@ static inline bool dw_mci_fifo_reset(struct dw_mci 
*host)
host-sg = NULL;
}
 
-   return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
-}
+   if (host-use_dma)
+   flags |= SDMMC_CTRL_DMA_RESET;
 
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
-{
-   return dw_mci_ctrl_reset(host,
-SDMMC_CTRL_FIFO_RESET |
-SDMMC_CTRL_RESET |
-SDMMC_CTRL_DMA_RESET);
+   if (dw_mci_ctrl_reset(host, flags)) {
+   /*
+* In all cases we clear the RAWINTS register to clear any
+* interrupts.
+*/
+   mci_writel(host, RINTSTS, 0x);
+
+   /* if using dma we wait for dma_req to clear */
+   if (host-use_dma) {
+   unsigned long timeout = jiffies + msecs_to_jiffies(500);
+   u32 status;
+   do {
+   status = mci_readl(host, STATUS);
+   if (!(status  SDMMC_STATUS_DMA_REQ))
+   break;
+   cpu_relax

Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure

2014-07-10 Thread Sonny Rao

On Thu, Jul 10, 2014 at 5:28 AM, Seungwon Jeon tgih@samsung.com wrote:
 Hi Sonny,

 I have missed this patch.

 You finally choose to take extra interrupt handling.
 If it is not harm, it's fine.

Hi, thanks for coming back to it.  Based on my tracing, the interrupt
seems to be okay and is just ignored.


 Please check one thing below.

 On Tue, June 10, 2014, Sonny Rao wrote:
 This patch changes the fifo reset code to follow the reset procedure
 outlined in the documentation of Synopsys Mobile storage host databook.

 Signed-off-by: Sonny Rao sonny...@chromium.org
 Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
 ---
 v2: Add Generic DMA support
 per the documentation, move interrupt clear before wait
 make the test for DMA host-use_dma rather than host-using_dma
 add proper return values (although it appears no caller checks)
 v3: rename fifo reset function, and change callers
 use this combined reset function in dw_mci_resume()
 just one caller left (probe), so get rid of dw_mci_ctrl_all_reset()
 use DMA reset bit for all systems which use DMA
 remove extra IDMAC reset in dw_mci_work_routine_card()
 do CIU clock update in error path, if CIU reset cleared
 v4: remove comment about FIFO reset in dw_mci_work_routine_card()
 move down error message when control reset clears but others don't
  and clarify the error stating that we will still update clocks
 make flags for all reset bits a macro

  drivers/mmc/host/dw_mmc.c | 86 
 ++-
  drivers/mmc/host/dw_mmc.h |  5 +++
  2 files changed, 68 insertions(+), 23 deletions(-)

 diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
 index 55cd110..1d6d984 100644
 --- a/drivers/mmc/host/dw_mmc.c
 +++ b/drivers/mmc/host/dw_mmc.c
 @@ -111,8 +111,7 @@ static const u8 tuning_blk_pattern_8bit[] = {
   0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
  };

 -static inline bool dw_mci_fifo_reset(struct dw_mci *host);
 -static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host);
 +static inline bool dw_mci_reset(struct dw_mci *host);

  #if defined(CONFIG_DEBUG_FS)
  static int dw_mci_req_show(struct seq_file *s, void *v)
 @@ -1254,7 +1253,7 @@ static int dw_mci_data_complete(struct dw_mci *host, 
 struct mmc_data *data)
* After an error, there may be data lingering
* in the FIFO
*/
 - dw_mci_fifo_reset(host);
 + dw_mci_reset(host);
   } else {
   data-bytes_xfered = data-blocks * data-blksz;
   data-error = 0;
 @@ -1371,7 +1370,7 @@ static void dw_mci_tasklet_func(unsigned long priv)

   /* CMD error in data command */
   if (mrq-cmd-error  mrq-data)
 - dw_mci_fifo_reset(host);
 + dw_mci_reset(host);

   host-cmd = NULL;
   host-data = NULL;
 @@ -1982,14 +1981,8 @@ static void dw_mci_work_routine_card(struct 
 work_struct *work)
   }

   /* Power down slot */
 - if (present == 0) {
 - /* Clear down the FIFO */
 - dw_mci_fifo_reset(host);
 -#ifdef CONFIG_MMC_DW_IDMAC
 - dw_mci_idmac_reset(host);
 -#endif
 -
 - }
 + if (present == 0)
 + dw_mci_reset(host);

   spin_unlock_bh(host-lock);

 @@ -2323,8 +2316,11 @@ static bool dw_mci_ctrl_reset(struct dw_mci *host, 
 u32 reset)
   return false;
  }

 -static inline bool dw_mci_fifo_reset(struct dw_mci *host)
 +static inline bool dw_mci_reset(struct dw_mci *host)
  {
 + u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
 + bool ret = false;
 +
   /*
* Reseting generates a block interrupt, hence setting
* the scatter-gather pointer to NULL.
 @@ -2334,15 +2330,59 @@ static inline bool dw_mci_fifo_reset(struct dw_mci 
 *host)
   host-sg = NULL;
   }

 - return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
 -}
 + if (host-use_dma)
 + flags |= SDMMC_CTRL_DMA_RESET;

 -static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
 -{
 - return dw_mci_ctrl_reset(host,
 -  SDMMC_CTRL_FIFO_RESET |
 -  SDMMC_CTRL_RESET |
 -  SDMMC_CTRL_DMA_RESET);
 + if (dw_mci_ctrl_reset(host, flags)) {
 + /*
 +  * In all cases we clear the RAWINTS register to clear any
 +  * interrupts.
 +  */
 + mci_writel(host, RINTSTS, 0x);
 +
 + /* if using dma we wait for dma_req to clear */
 + if (host-use_dma) {
 + unsigned long timeout = jiffies + 
 msecs_to_jiffies(500);
 + u32 status

Using s5p-sss hw crypto causes ipsec to break

2014-06-23 Thread Sonny Rao

Hi, I've been investigating an issue relating to hardware crypto which
is that when I enable the s5p-sss module for hardware cryptographic
acceleration on Samsung Exynos SoCs the in-kernel IPSec seems to
break, although cryptographic operations on filesystems/block devices
using this driver seem to work fine.

Originally we were looking at an older kernel (3.8 with patches), but
I've now verified on linux-next from 20140612 (after 3.15) with a few
additional patches (to enable both s5p-sss and Exynos5420) that this
is still the case.

It looks like what is happening in the kernel is that IPsec ends up
with this callstack

esp_output()- crypto_aead_givencrypt()-
crypto_authenc_givencrypt()- eseqiv_givencrypt() -
crypto_ablkcipher_encrypt()


which calls into the s5p-sss driver and that is returning -EINPROGRESS
(or possibly -EBUSY), and that is passed all the way back up the call
stack and that seems to be treated as an error condition by the ipsec
stack.

For example esp_output does this:

   err = crypto_aead_givencrypt(req);
if (err == -EINPROGRESS)
goto error;

if (err == -EBUSY)
err = NET_XMIT_DROP;

So, I'm not sure how this is supposed to work, or if s5p-sss is doing
something wrong.

In the case of the block layer, it seems to be tolerant of the
-EINPROGRESS return code.  I looked at some of the other hw crypto
drivers which are similar to s5p-sss and they also seem to return
-EINPROGRESS or -EBUSY in a similar way.

I was also wondering if esp shouldn't be using the ablkcipher
interface if it isn't tolerant of the -EINPROGRESS?
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc 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: dw_mmc: change to use recommended reset procedure

2014-06-09 Thread Sonny Rao

On Wed, May 28, 2014 at 10:17 PM, Jaehoon Chung jh80.ch...@samsung.com wrote:
 Hi, Sonny.

 On 05/29/2014 09:35 AM, Sonny Rao wrote:
 This patch changes the fifo reset code to follow the reset procedure
 outlined in the documentation of Synopsys Mobile storage host databook.

 Signed-off-by: Sonny Rao sonny...@chromium.org
 Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
 ---
 v2: Add Generic DMA support
 per the documentation, move interrupt clear before wait
 make the test for DMA host-use_dma rather than host-using_dma
 add proper return values (although it appears no caller checks)
 v3: rename fifo reset function, and change callers
 use this combined reset function in dw_mci_resume()
 just one caller left (probe), so get rid of dw_mci_ctrl_all_reset()
 use DMA reset bit for all systems which use DMA
 remove extra IDMAC reset in dw_mci_work_routine_card()
 do CIU clock update in error path, if CIU reset cleared

  drivers/mmc/host/dw_mmc.c | 85 
 +++
  drivers/mmc/host/dw_mmc.h |  1 +
  2 files changed, 64 insertions(+), 22 deletions(-)

 diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
 index 55cd110..988492c 100644
 --- a/drivers/mmc/host/dw_mmc.c
 +++ b/drivers/mmc/host/dw_mmc.c
 @@ -111,8 +111,7 @@ static const u8 tuning_blk_pattern_8bit[] = {
   0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
  };

 -static inline bool dw_mci_fifo_reset(struct dw_mci *host);
 -static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host);
 +static inline bool dw_mci_reset(struct dw_mci *host);

  #if defined(CONFIG_DEBUG_FS)
  static int dw_mci_req_show(struct seq_file *s, void *v)
 @@ -1254,7 +1253,7 @@ static int dw_mci_data_complete(struct dw_mci *host, 
 struct mmc_data *data)
* After an error, there may be data lingering
* in the FIFO
*/
 - dw_mci_fifo_reset(host);
 + dw_mci_reset(host);
   } else {
   data-bytes_xfered = data-blocks * data-blksz;
   data-error = 0;
 @@ -1371,7 +1370,7 @@ static void dw_mci_tasklet_func(unsigned long priv)

   /* CMD error in data command */
   if (mrq-cmd-error  mrq-data)
 - dw_mci_fifo_reset(host);
 + dw_mci_reset(host);

   host-cmd = NULL;
   host-data = NULL;
 @@ -1982,14 +1981,9 @@ static void dw_mci_work_routine_card(struct 
 work_struct *work)
   }

   /* Power down slot */
 - if (present == 0) {
 + if (present == 0)
   /* Clear down the FIFO */
 Didn't Need to change the Comment?

Well, it does still clear the fifo, but I can remove the comment since
we are now doing more than that.

 - dw_mci_fifo_reset(host);
 -#ifdef CONFIG_MMC_DW_IDMAC
 - dw_mci_idmac_reset(host);
 -#endif
 -
 - }
 + dw_mci_reset(host);

   spin_unlock_bh(host-lock);

 @@ -2323,8 +2317,11 @@ static bool dw_mci_ctrl_reset(struct dw_mci *host, 
 u32 reset)
   return false;
  }

 -static inline bool dw_mci_fifo_reset(struct dw_mci *host)
 +static inline bool dw_mci_reset(struct dw_mci *host)
  {
 + u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
 + bool ret = false;
 +
   /*
* Reseting generates a block interrupt, hence setting
* the scatter-gather pointer to NULL.
 @@ -2334,15 +2331,57 @@ static inline bool dw_mci_fifo_reset(struct dw_mci 
 *host)
   host-sg = NULL;
   }

 - return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
 -}
 + if (host-use_dma)
 + flags |= SDMMC_CTRL_DMA_RESET;

 -static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
 -{
 - return dw_mci_ctrl_reset(host,
 -  SDMMC_CTRL_FIFO_RESET |
 -  SDMMC_CTRL_RESET |
 -  SDMMC_CTRL_DMA_RESET);
 + if (dw_mci_ctrl_reset(host, flags)) {
 + /*
 +  * In all cases we clear the RAWINTS register to clear any
 +  * interrupts.
 +  */
 + mci_writel(host, RINTSTS, 0x);
 +
 + /* if using dma we wait for dma_req to clear */
 + if (host-use_dma) {
 + unsigned long timeout = jiffies + 
 msecs_to_jiffies(500);
 + u32 status;
 + do {
 + status = mci_readl(host, STATUS);
 + if (!(status  SDMMC_STATUS_DMA_REQ))
 + break;
 + cpu_relax();
 + } while (time_before(jiffies, timeout));
 +
 + if (status

[PATCH] mmc: dw_mmc: change to use recommended reset procedure

2014-06-09 Thread Sonny Rao

This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook.

Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
---
v2: Add Generic DMA support
per the documentation, move interrupt clear before wait
make the test for DMA host-use_dma rather than host-using_dma
add proper return values (although it appears no caller checks)
v3: rename fifo reset function, and change callers
use this combined reset function in dw_mci_resume()
just one caller left (probe), so get rid of dw_mci_ctrl_all_reset()
use DMA reset bit for all systems which use DMA
remove extra IDMAC reset in dw_mci_work_routine_card()
do CIU clock update in error path, if CIU reset cleared
v4: remove comment about FIFO reset in dw_mci_work_routine_card()
move down error message when control reset clears but others don't
 and clarify the error stating that we will still update clocks
make flags for all reset bits a macro

 drivers/mmc/host/dw_mmc.c | 86 ++-
 drivers/mmc/host/dw_mmc.h |  5 +++
 2 files changed, 68 insertions(+), 23 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 55cd110..1d6d984 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -111,8 +111,7 @@ static const u8 tuning_blk_pattern_8bit[] = {
0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
 };
 
-static inline bool dw_mci_fifo_reset(struct dw_mci *host);
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host);
+static inline bool dw_mci_reset(struct dw_mci *host);
 
 #if defined(CONFIG_DEBUG_FS)
 static int dw_mci_req_show(struct seq_file *s, void *v)
@@ -1254,7 +1253,7 @@ static int dw_mci_data_complete(struct dw_mci *host, 
struct mmc_data *data)
 * After an error, there may be data lingering
 * in the FIFO
 */
-   dw_mci_fifo_reset(host);
+   dw_mci_reset(host);
} else {
data-bytes_xfered = data-blocks * data-blksz;
data-error = 0;
@@ -1371,7 +1370,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
 
/* CMD error in data command */
if (mrq-cmd-error  mrq-data)
-   dw_mci_fifo_reset(host);
+   dw_mci_reset(host);
 
host-cmd = NULL;
host-data = NULL;
@@ -1982,14 +1981,8 @@ static void dw_mci_work_routine_card(struct work_struct 
*work)
}
 
/* Power down slot */
-   if (present == 0) {
-   /* Clear down the FIFO */
-   dw_mci_fifo_reset(host);
-#ifdef CONFIG_MMC_DW_IDMAC
-   dw_mci_idmac_reset(host);
-#endif
-
-   }
+   if (present == 0)
+   dw_mci_reset(host);
 
spin_unlock_bh(host-lock);
 
@@ -2323,8 +2316,11 @@ static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 
reset)
return false;
 }
 
-static inline bool dw_mci_fifo_reset(struct dw_mci *host)
+static inline bool dw_mci_reset(struct dw_mci *host)
 {
+   u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
+   bool ret = false;
+
/*
 * Reseting generates a block interrupt, hence setting
 * the scatter-gather pointer to NULL.
@@ -2334,15 +2330,59 @@ static inline bool dw_mci_fifo_reset(struct dw_mci 
*host)
host-sg = NULL;
}
 
-   return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
-}
+   if (host-use_dma)
+   flags |= SDMMC_CTRL_DMA_RESET;
 
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
-{
-   return dw_mci_ctrl_reset(host,
-SDMMC_CTRL_FIFO_RESET |
-SDMMC_CTRL_RESET |
-SDMMC_CTRL_DMA_RESET);
+   if (dw_mci_ctrl_reset(host, flags)) {
+   /*
+* In all cases we clear the RAWINTS register to clear any
+* interrupts.
+*/
+   mci_writel(host, RINTSTS, 0x);
+
+   /* if using dma we wait for dma_req to clear */
+   if (host-use_dma) {
+   unsigned long timeout = jiffies + msecs_to_jiffies(500);
+   u32 status;
+   do {
+   status = mci_readl(host, STATUS);
+   if (!(status  SDMMC_STATUS_DMA_REQ))
+   break;
+   cpu_relax();
+   } while (time_before(jiffies, timeout));
+
+   if (status  SDMMC_STATUS_DMA_REQ

[PATCH] mmc: dw_mmc: change to use recommended reset procedure

2014-05-28 Thread Sonny Rao

This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook.

Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
---
v2: Add Generic DMA support
per the documentation, move interrupt clear before wait
make the test for DMA host-use_dma rather than host-using_dma
add proper return values (although it appears no caller checks)
v3: rename fifo reset function, and change callers
use this combined reset function in dw_mci_resume()
just one caller left (probe), so get rid of dw_mci_ctrl_all_reset()
use DMA reset bit for all systems which use DMA
remove extra IDMAC reset in dw_mci_work_routine_card()
do CIU clock update in error path, if CIU reset cleared

 drivers/mmc/host/dw_mmc.c | 85 +++
 drivers/mmc/host/dw_mmc.h |  1 +
 2 files changed, 64 insertions(+), 22 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 55cd110..988492c 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -111,8 +111,7 @@ static const u8 tuning_blk_pattern_8bit[] = {
0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
 };
 
-static inline bool dw_mci_fifo_reset(struct dw_mci *host);
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host);
+static inline bool dw_mci_reset(struct dw_mci *host);
 
 #if defined(CONFIG_DEBUG_FS)
 static int dw_mci_req_show(struct seq_file *s, void *v)
@@ -1254,7 +1253,7 @@ static int dw_mci_data_complete(struct dw_mci *host, 
struct mmc_data *data)
 * After an error, there may be data lingering
 * in the FIFO
 */
-   dw_mci_fifo_reset(host);
+   dw_mci_reset(host);
} else {
data-bytes_xfered = data-blocks * data-blksz;
data-error = 0;
@@ -1371,7 +1370,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
 
/* CMD error in data command */
if (mrq-cmd-error  mrq-data)
-   dw_mci_fifo_reset(host);
+   dw_mci_reset(host);
 
host-cmd = NULL;
host-data = NULL;
@@ -1982,14 +1981,9 @@ static void dw_mci_work_routine_card(struct work_struct 
*work)
}
 
/* Power down slot */
-   if (present == 0) {
+   if (present == 0)
/* Clear down the FIFO */
-   dw_mci_fifo_reset(host);
-#ifdef CONFIG_MMC_DW_IDMAC
-   dw_mci_idmac_reset(host);
-#endif
-
-   }
+   dw_mci_reset(host);
 
spin_unlock_bh(host-lock);
 
@@ -2323,8 +2317,11 @@ static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 
reset)
return false;
 }
 
-static inline bool dw_mci_fifo_reset(struct dw_mci *host)
+static inline bool dw_mci_reset(struct dw_mci *host)
 {
+   u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
+   bool ret = false;
+
/*
 * Reseting generates a block interrupt, hence setting
 * the scatter-gather pointer to NULL.
@@ -2334,15 +2331,57 @@ static inline bool dw_mci_fifo_reset(struct dw_mci 
*host)
host-sg = NULL;
}
 
-   return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
-}
+   if (host-use_dma)
+   flags |= SDMMC_CTRL_DMA_RESET;
 
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
-{
-   return dw_mci_ctrl_reset(host,
-SDMMC_CTRL_FIFO_RESET |
-SDMMC_CTRL_RESET |
-SDMMC_CTRL_DMA_RESET);
+   if (dw_mci_ctrl_reset(host, flags)) {
+   /*
+* In all cases we clear the RAWINTS register to clear any
+* interrupts.
+*/
+   mci_writel(host, RINTSTS, 0x);
+
+   /* if using dma we wait for dma_req to clear */
+   if (host-use_dma) {
+   unsigned long timeout = jiffies + msecs_to_jiffies(500);
+   u32 status;
+   do {
+   status = mci_readl(host, STATUS);
+   if (!(status  SDMMC_STATUS_DMA_REQ))
+   break;
+   cpu_relax();
+   } while (time_before(jiffies, timeout));
+
+   if (status  SDMMC_STATUS_DMA_REQ) {
+   dev_err(host-dev,
+   %s: Timeout waiting for dma_req to 
+   clear during reset, __func__);
+   goto ciu_out

Re: [PATCHv2] mmc: dw_mmc: change to use recommended reset procedure

2014-05-22 Thread Sonny Rao

On Mon, May 19, 2014 at 6:49 PM, Seungwon Jeon tgih@samsung.com wrote:
 On Sat, May 17, 2014, Sonny Rao wrote:
 On Tue, May 13, 2014 at 4:09 AM, Seungwon Jeon tgih@samsung.com wrote:
  On Tuesday, May 13, Sonny Rao wrote:
  On Mon, May 12, 2014 at 10:02 PM, Seungwon Jeon tgih@samsung.com 
  wrote:
   As I mentioned in previous version, you put all reset stuff in existing 
   fifo_reset function.
   Although databook mentions ciu_reset case for SBE error, it's not 
   obvious when ciu_reset is
 needed
  in other error cases.
   If you intend to add some robust error handing, it would be better to 
   make another function.
 
  The document I have actually doesn't mention error cases, it describes
  this procedure as Controller/DMA/FIFO Reset Usage so I believe it is
  saying this is the correct procedure in all cases, and based on our
  testing it seems to work.  I understand the skepticism, as I shared it
  initially when I saw this, but based on our experience, this is
  correct and it doesn't need to live in a separate function.
  I agree this active error handling.
  But, existing fifo_reset function is focused on fifo reset purely.
  I think your change is close to error recovery and it seems overloaded to 
  basic function.
  So, you suggest renaming function for new sequence.

 I think the documentation says it should always be done, not just in
 error recovery.  I can rename the function to dw_mci_reset rather than
 dw_mci_fifo_reset.  Is that what you mean? Or do you mean make a new
 function that is only called in error cases?
 Both are okay.


Ok


  And look into dw_mci_work_routine_card(). dw_mci_idmac_reset() is 
  redundancy.
  I expect it can be cleaned.
 
  Quot
  /* Clear down the FIFO */
  dw_mci_fifo_reset(host);
  #ifdef CONFIG_MMC_DW_IDMAC
  dw_mci_idmac_reset(host);
  #endif
  /Quot
 

 Ok, I'll remove that extra reset, thanks for catching.

 
   Please check my comments below.
  
   On Tue, May 13, 2014, Sonny Rao wrote:
   This patch changes the fifo reset code to follow the reset procedure
   outlined in the documentation of Synopsys  Mobile storage host databook
   7.2.13.
  Please remove this section number.
  No needed. It's old version.
 

 Ok

  
   v2: Add Generic DMA support
   per the documentation, move interrupt clear before wait
   make the test for DMA host-use_dma rather than host-using_dma
   add proper return values (although it appears no caller checks)
  
   Signed-off-by: Sonny Rao sonny...@chromium.org
   Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
   ---
drivers/mmc/host/dw_mmc.c | 55 
   ++-
drivers/mmc/host/dw_mmc.h |  1 +
2 files changed, 55 insertions(+), 1 deletion(-)
  
   diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
   index 55cd110..aff57e1 100644
   --- a/drivers/mmc/host/dw_mmc.c
   +++ b/drivers/mmc/host/dw_mmc.c
   @@ -2325,6 +2325,7 @@ static bool dw_mci_ctrl_reset(struct dw_mci 
   *host, u32 reset)
  
static inline bool dw_mci_fifo_reset(struct dw_mci *host)
{
   + u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
 /*
  * Reseting generates a block interrupt, hence setting
  * the scatter-gather pointer to NULL.
   @@ -2334,7 +2335,59 @@ static inline bool dw_mci_fifo_reset(struct 
   dw_mci *host)
 host-sg = NULL;
 }
  
   - return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
   + /*
   +  * The recommended method for resetting is to always reset the
   +  * controller and the fifo, but differs slightly depending on 
   the mode.
   +  * The Generic DMA mode (non IDMAC) also needs to reset DMA 
   where IDMAC
   +  * mode resets IDMAC at the end.
   +  *
   +  */
   +#ifndef CONFIG_MMC_DW_IDMAC
   Is it added for generic DMA?
   IDMAC should be considered for dma_reseet as well.
   Please check databook.
  
 
  Yeah it's a little unclear.  In the 7.2.13 Controller/DMA/FIFO Reset
  Usage part of the document they mention It is set for what they call
  generic DMA, which I think is when there is an external DMA
  controller, and the part below that it says for DW-DMA/Non-DW-DMA
  that controller_reset and fifo_reset should be set.   I believe this
  DW-DMA case refers to what is called IDMAC.  So, I think it's not
  required for this case, but I admit I'm not sure why they also say
  Non-DW-DMA. If you know of a good way to differentiate the Generic
  DMA case  I can implement it.  It wasn't clear to me if the driver
  even supported this generic dma case, but it sounds like it might,
  so I added this code.  In practice, on the Exynos Systems we have,
  which are using IDMAC, the reset procedure seems to work okay without
  the dma_reset bit set, but I cannot test this generic dma case.
 
  The other places in the doc where I see them mention

Re: [PATCHv2] mmc: dw_mmc: change to use recommended reset procedure

2014-05-16 Thread Sonny Rao

On Tue, May 13, 2014 at 4:09 AM, Seungwon Jeon tgih@samsung.com wrote:
 On Tuesday, May 13, Sonny Rao wrote:
 On Mon, May 12, 2014 at 10:02 PM, Seungwon Jeon tgih@samsung.com wrote:
  As I mentioned in previous version, you put all reset stuff in existing 
  fifo_reset function.
  Although databook mentions ciu_reset case for SBE error, it's not obvious 
  when ciu_reset is needed
 in other error cases.
  If you intend to add some robust error handing, it would be better to make 
  another function.

 The document I have actually doesn't mention error cases, it describes
 this procedure as Controller/DMA/FIFO Reset Usage so I believe it is
 saying this is the correct procedure in all cases, and based on our
 testing it seems to work.  I understand the skepticism, as I shared it
 initially when I saw this, but based on our experience, this is
 correct and it doesn't need to live in a separate function.
 I agree this active error handling.
 But, existing fifo_reset function is focused on fifo reset purely.
 I think your change is close to error recovery and it seems overloaded to 
 basic function.
 So, you suggest renaming function for new sequence.

I think the documentation says it should always be done, not just in
error recovery.  I can rename the function to dw_mci_reset rather than
dw_mci_fifo_reset.  Is that what you mean? Or do you mean make a new
function that is only called in error cases?

 And look into dw_mci_work_routine_card(). dw_mci_idmac_reset() is redundancy.
 I expect it can be cleaned.

 Quot
 /* Clear down the FIFO */
 dw_mci_fifo_reset(host);
 #ifdef CONFIG_MMC_DW_IDMAC
 dw_mci_idmac_reset(host);
 #endif
 /Quot


Ok, I'll remove that extra reset, thanks for catching.


  Please check my comments below.
 
  On Tue, May 13, 2014, Sonny Rao wrote:
  This patch changes the fifo reset code to follow the reset procedure
  outlined in the documentation of Synopsys  Mobile storage host databook
  7.2.13.
 Please remove this section number.
 No needed. It's old version.


Ok

 
  v2: Add Generic DMA support
  per the documentation, move interrupt clear before wait
  make the test for DMA host-use_dma rather than host-using_dma
  add proper return values (although it appears no caller checks)
 
  Signed-off-by: Sonny Rao sonny...@chromium.org
  Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
  ---
   drivers/mmc/host/dw_mmc.c | 55 
  ++-
   drivers/mmc/host/dw_mmc.h |  1 +
   2 files changed, 55 insertions(+), 1 deletion(-)
 
  diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
  index 55cd110..aff57e1 100644
  --- a/drivers/mmc/host/dw_mmc.c
  +++ b/drivers/mmc/host/dw_mmc.c
  @@ -2325,6 +2325,7 @@ static bool dw_mci_ctrl_reset(struct dw_mci *host, 
  u32 reset)
 
   static inline bool dw_mci_fifo_reset(struct dw_mci *host)
   {
  + u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
/*
 * Reseting generates a block interrupt, hence setting
 * the scatter-gather pointer to NULL.
  @@ -2334,7 +2335,59 @@ static inline bool dw_mci_fifo_reset(struct dw_mci 
  *host)
host-sg = NULL;
}
 
  - return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
  + /*
  +  * The recommended method for resetting is to always reset the
  +  * controller and the fifo, but differs slightly depending on the 
  mode.
  +  * The Generic DMA mode (non IDMAC) also needs to reset DMA where 
  IDMAC
  +  * mode resets IDMAC at the end.
  +  *
  +  */
  +#ifndef CONFIG_MMC_DW_IDMAC
  Is it added for generic DMA?
  IDMAC should be considered for dma_reseet as well.
  Please check databook.
 

 Yeah it's a little unclear.  In the 7.2.13 Controller/DMA/FIFO Reset
 Usage part of the document they mention It is set for what they call
 generic DMA, which I think is when there is an external DMA
 controller, and the part below that it says for DW-DMA/Non-DW-DMA
 that controller_reset and fifo_reset should be set.   I believe this
 DW-DMA case refers to what is called IDMAC.  So, I think it's not
 required for this case, but I admit I'm not sure why they also say
 Non-DW-DMA. If you know of a good way to differentiate the Generic
 DMA case  I can implement it.  It wasn't clear to me if the driver
 even supported this generic dma case, but it sounds like it might,
 so I added this code.  In practice, on the Exynos Systems we have,
 which are using IDMAC, the reset procedure seems to work okay without
 the dma_reset bit set, but I cannot test this generic dma case.

 The other places in the doc where I see them mention the dma_reset bit
 are 7.2.21 Transmission and Reception with Internal DMAC (IDMAC)
 and the description of the CTRL register, and in 7.1
 Software/Hardware Restrictions where they say it will terminate any
 pending transfer.
 DW-DMA means Synopsys's DMA

Re: [PATCH] arm: dts: exynos5: Remove multi core timer

2014-05-15 Thread Sonny Rao

On Thu, May 15, 2014 at 4:43 PM, Doug Anderson diand...@chromium.org wrote:
 Tomasz,

 On Thu, May 15, 2014 at 4:25 PM, Tomasz Figa tomasz.f...@gmail.com wrote:
 On 16.05.2014 01:18, David Riley wrote:
 On Thu, May 15, 2014 at 4:03 PM, Chirantan Ekbote
 chiran...@chromium.org wrote:
 Hi Tomasz,

 On Thu, May 15, 2014 at 3:44 PM, Doug Anderson diand...@chromium.org 
 wrote:
 Tomasz,

 On Thu, May 15, 2014 at 3:13 PM, Tomasz Figa tomasz.f...@gmail.com 
 wrote:
 NOTE: if for some reason we need to keep the MCT around, we're
 definitely going to need to account for the fact that tweaking it
 affects the arch timer.  ...and having the arch timer is really nice
 since:

 [Let me reorder the points below to make it easier to comment:]

 * it's faster to access.
 * it is accessible from userspace for really fast access.

 Do you have some data on whether it is a significant difference,
 especially considering real use cases?

 I know that Chrome makes _a lot_ of calls to gettimeofday() for
 profiling purposes, enough that it showed up on benchmarks.  In fact,
 we made a change to the MCT to make accesses faster and there's a
 small mention of the benchmarking that was done at:

 https://chromium-review.googlesource.com/#/c/32190/

 ...that change probably should be sent upstream, actually.

 I'll let Chirantan comment on how much faster arch timers were.
 ...and I think David Riley (also CCed now) may be able to comment on
 the benefits of userspace timers.


 When I profiled gettimeofday() calls, they were about 50 - 60% faster
 with the arch timers compared to the mct.

 When I profiled gettimeofday(), the standard systems call version took
 about 2.5x longer than through a vDSO interface.

 Sounds like we should invent a new kind of jokes, starting with When I
 profiled gettimeofday(). ;)

 Just kidding.

 The raw improvement looks quite good, but what I'm more concerned about
 is whether this has any significant effect on real use cases. As Doug
 mentioned, Chrome makes a lot of calls to gettimeofday(), but he also
 mentioned that this is for profiling purposes. Is performance of
 gettimeofday() really that crucial in this case? Are there any other use
 cases when this improvement is significant?

 I guess I should restate.  Chrome is always profiling to some extent.
 I think that the Javascript engine, for instance, uses gettimeofday()
 to figure out how much time it's spending in various places.

 Sonny just sent me some recent benchmarks using perf.  On this
 particular benchmark it shows 1.85% of the time was spent in
 exynos_frc_read.  If we can half that then we'll get a ~1% speedup in
 the system, which is nothing to sneeze at.  If getting rid of the
 system call overead makes this several times faster again, then we
 roughly eliminate the overhead totally.

To clarify, that data wasn't a benchmark -- It's field data, so
actually much better than a benchmark.



 Anyway, I'm by no means opposed to switching to arch timers. They
 provide a well designed, generic interface and drivers shared by
 multiple platforms, which means more code sharing and possibly more eyes
 looking at the code, which is always good. However if they don't support
 low power states correctly, we can't just remove MCT.

 I think low power states aren't in mainline (right?).

 One solution that might work could be to leave the device tree entry
 alone but change the MCT init code to simply act as a no-op if it sees
 an arch timer is in the device tree and enabled.  Then when/if someone
 got the low power states enabled we could just change source code
 rather than dts files.

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

Re: [PATCHv2] mmc: dw_mmc: change to use recommended reset procedure

2014-05-13 Thread Sonny Rao

On Mon, May 12, 2014 at 10:02 PM, Seungwon Jeon tgih@samsung.com wrote:
 As I mentioned in previous version, you put all reset stuff in existing 
 fifo_reset function.
 Although databook mentions ciu_reset case for SBE error, it's not obvious 
 when ciu_reset is needed in other error cases.
 If you intend to add some robust error handing, it would be better to make 
 another function.

The document I have actually doesn't mention error cases, it describes
this procedure as Controller/DMA/FIFO Reset Usage so I believe it is
saying this is the correct procedure in all cases, and based on our
testing it seems to work.  I understand the skepticism, as I shared it
initially when I saw this, but based on our experience, this is
correct and it doesn't need to live in a separate function.

 Please check my comments below.

 On Tue, May 13, 2014, Sonny Rao wrote:
 This patch changes the fifo reset code to follow the reset procedure
 outlined in the documentation of Synopsys  Mobile storage host databook
 7.2.13.

 v2: Add Generic DMA support
 per the documentation, move interrupt clear before wait
 make the test for DMA host-use_dma rather than host-using_dma
 add proper return values (although it appears no caller checks)

 Signed-off-by: Sonny Rao sonny...@chromium.org
 Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
 ---
  drivers/mmc/host/dw_mmc.c | 55 
 ++-
  drivers/mmc/host/dw_mmc.h |  1 +
  2 files changed, 55 insertions(+), 1 deletion(-)

 diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
 index 55cd110..aff57e1 100644
 --- a/drivers/mmc/host/dw_mmc.c
 +++ b/drivers/mmc/host/dw_mmc.c
 @@ -2325,6 +2325,7 @@ static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 
 reset)

  static inline bool dw_mci_fifo_reset(struct dw_mci *host)
  {
 + u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
   /*
* Reseting generates a block interrupt, hence setting
* the scatter-gather pointer to NULL.
 @@ -2334,7 +2335,59 @@ static inline bool dw_mci_fifo_reset(struct dw_mci 
 *host)
   host-sg = NULL;
   }

 - return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
 + /*
 +  * The recommended method for resetting is to always reset the
 +  * controller and the fifo, but differs slightly depending on the mode.
 +  * The Generic DMA mode (non IDMAC) also needs to reset DMA where IDMAC
 +  * mode resets IDMAC at the end.
 +  *
 +  */
 +#ifndef CONFIG_MMC_DW_IDMAC
 Is it added for generic DMA?
 IDMAC should be considered for dma_reseet as well.
 Please check databook.


Yeah it's a little unclear.  In the 7.2.13 Controller/DMA/FIFO Reset
Usage part of the document they mention It is set for what they call
generic DMA, which I think is when there is an external DMA
controller, and the part below that it says for DW-DMA/Non-DW-DMA
that controller_reset and fifo_reset should be set.   I believe this
DW-DMA case refers to what is called IDMAC.  So, I think it's not
required for this case, but I admit I'm not sure why they also say
Non-DW-DMA. If you know of a good way to differentiate the Generic
DMA case  I can implement it.  It wasn't clear to me if the driver
even supported this generic dma case, but it sounds like it might,
so I added this code.  In practice, on the Exynos Systems we have,
which are using IDMAC, the reset procedure seems to work okay without
the dma_reset bit set, but I cannot test this generic dma case.

The other places in the doc where I see them mention the dma_reset bit
are 7.2.21 Transmission and Reception with Internal DMAC (IDMAC)
and the description of the CTRL register, and in 7.1
Software/Hardware Restrictions where they say it will terminate any
pending transfer.

 + if (host-use_dma)
 + flags |= SDMMC_CTRL_DMA_RESET;
 +#endif
 + if (dw_mci_ctrl_reset(host, flags)) {
 + /*
 +  * In all cases we clear the RAWINTS register to clear any
 +  * interrupts.
 +  */
 I think interrupt masking is needed before reset because we will not handle 
 it anymore.
 And Is there any reason to move interrupt clear here compared with previous 
 version?

Yeah I moved it to match the description in the document more closely.
 The documents mentioned doing the interrupt clear after setting the
reset bits, and before waiting for the dma_req bit in the status
register to clear.  We've been running it with the interrupt clear
below the loop, for a while, and I just tested it with the clear above
the wait to make sure it still works properly and I was able to pass
the tuning process with some errors, so I believe this works fine too,
and more closely matches the description in the doc that I have.

 + mci_writel(host, RINTSTS, 0x);
 +
 + /* if using dma we wait for dma_req to clear */
 + if (host-use_dma) {
 + unsigned long timeout

Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure

2014-05-12 Thread Sonny Rao

On Sat, May 10, 2014 at 7:08 AM, Seungwon Jeon tgih@samsung.com wrote:
 Hi Sonny,

 Can you separate procedure?
 Reset all are handled in fifo-reset.
 And ciu reset is always needed for error handling?

Yes according to the document in the Controller/DMA/FIFO Reset Usage
section, the controller_reset bit should be set in all cases, so I
don't think that it should be separated.


 Thanks,
 Seungwon Jeon

 On Sat, May 10, 2014, Sonny Rao wrote:
 On Fri, May 9, 2014 at 12:32 AM, Jaehoon Chung jh80.ch...@samsung.com 
 wrote:
  Hi, Sonny.
 
  You can discard the my previous some comment.
  As you mentioned, this reset sequence is recommended at Synopsys TRM.
 
  Add the minor question.
 
  On 05/09/2014 01:27 PM, Jaehoon Chung wrote:
  Hi, Sonny.
 
  I have checked the Synopsys TRM..
 
  On 05/09/2014 10:34 AM, Sonny Rao wrote:
  On Thu, May 8, 2014 at 6:15 PM, Jaehoon Chung jh80.ch...@samsung.com 
  wrote:
  On 05/08/2014 06:40 PM, Yuvaraj Kumar wrote:
  Any comments on this patch?
 
  On Wed, Mar 26, 2014 at 5:16 PM, Yuvaraj Kumar C D 
  yuvaraj...@gmail.com wrote:
  From: Sonny Rao sonny...@chromium.org
 
  This patch changes the fifo reset code to follow the reset procedure
  outlined in the documentation of Synopsys  Mobile storage host 
  databook
  7.2.13.
  Without this patch, we could able to see eMMC was not detected after
  multiple reboots due to driver hangs while eMMC tuning for HS200.
 
  Signed-off-by: Sonny Rao sonny...@chromium.org
  Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.org
  ---
   drivers/mmc/host/dw_mmc.c |   48 
  -
   drivers/mmc/host/dw_mmc.h |1 +
   2 files changed, 48 insertions(+), 1 deletion(-)
 
  diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
  index 32dd81d..1d77431 100644
  --- a/drivers/mmc/host/dw_mmc.c
  +++ b/drivers/mmc/host/dw_mmc.c
  @@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct 
  dw_mci *host)
  host-sg = NULL;
  }
 
  -   return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
  +   /*
  +* The recommended method for resetting is to always reset the
  +* controller and the fifo, but differs slightly depending on 
  the mode.
  +* Note that this doesn't handle the generic DMA (not 
  IDMAC) case.
  +*/
 
  not IDMAC is confused..
 
 
  The documentation describes three different possible modes.  There's a
  mode that doesn't use IDMAC but still does DMA.  But as far as I can
  tell this driver doesn't support that way.  We can just remove that
  wording if it's confusing.
 
  How did it know whether dma is generic DMA or not?
 

 That's a good question.  I wasn't sure whether the driver supported it
 or not.  It looks like it definitely supports IDMAC through the
 CONFIG_MMC_DW_IDMAC flag, but I wasn't sure if it was supported the
 generic dma.  Maybe if CONFIG_MMC_DW_IDMAC isn't specified but
 host-dma_ops is not NULL then we are using the generic dma mode.

 
  +   if (dw_mci_ctrl_reset(host, SDMMC_CTRL_RESET | 
  SDMMC_CTRL_FIFO_RESET)) {
  +   unsigned long timeout = jiffies + 
  msecs_to_jiffies(500);
  +   u32 status, rint;
  +
  +   /* if using dma we wait for dma_req to clear */
  +   if (host-using_dma) {
  +   do {
  +   status = mci_readl(host, STATUS);
  +   if (!(status  SDMMC_STATUS_DMA_REQ))
  +   break;
  +   cpu_relax();
  +   } while (time_before(jiffies, timeout));
  +
  +   if (status  SDMMC_STATUS_DMA_REQ)
  +   dev_err(host-dev,
  +   %s: Timeout waiting for 
  dma_req to 
  +   clear during reset, 
  __func__);
  +
  +   /* when using DMA next we reset the fifo 
  again */
  +   dw_mci_ctrl_reset(host, 
  SDMMC_CTRL_FIFO_RESET);
  +   }
  +   /*
  +* In all cases we clear the RAWINTS register to 
  clear any
  +* interrupts.
  +*/
  +   rint = mci_readl(host, RINTSTS);
  +   rint = rint  (~mci_readl(host, MINTSTS));
  you use the status or temp instead of rint. (you can reuse the 
  variable.)
  And can use status = ~mci_readl(host,MINTSTS);
 
 
  Just clear the RINTSTS register? why do you add these?
 
 
  This will look at what is not masked, and only clear those bits.
  Well, i known if clear the RINTSTS register,
  recommended to use 0xfff and set the value for interrupt.
 
  Can be used 0xfff?
 

 Yeah we probably can.  We just lose information about interrupts that
 were masked, but maybe we just don't care about any of them anyway, so
 it doesn't matter.

  Best Regards,
  Jaehoon Chung

Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure

2014-05-12 Thread Sonny Rao

On Fri, May 9, 2014 at 8:36 PM, Sonny Rao sonny...@chromium.org wrote:
 On Fri, May 9, 2014 at 12:32 AM, Jaehoon Chung jh80.ch...@samsung.com wrote:
 Hi, Sonny.

 You can discard the my previous some comment.
 As you mentioned, this reset sequence is recommended at Synopsys TRM.

 Add the minor question.

 On 05/09/2014 01:27 PM, Jaehoon Chung wrote:
 Hi, Sonny.

 I have checked the Synopsys TRM..

 On 05/09/2014 10:34 AM, Sonny Rao wrote:
 On Thu, May 8, 2014 at 6:15 PM, Jaehoon Chung jh80.ch...@samsung.com 
 wrote:
 On 05/08/2014 06:40 PM, Yuvaraj Kumar wrote:
 Any comments on this patch?

 On Wed, Mar 26, 2014 at 5:16 PM, Yuvaraj Kumar C D 
 yuvaraj...@gmail.com wrote:
 From: Sonny Rao sonny...@chromium.org

 This patch changes the fifo reset code to follow the reset procedure
 outlined in the documentation of Synopsys  Mobile storage host databook
 7.2.13.
 Without this patch, we could able to see eMMC was not detected after
 multiple reboots due to driver hangs while eMMC tuning for HS200.

 Signed-off-by: Sonny Rao sonny...@chromium.org
 Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.org
 ---
  drivers/mmc/host/dw_mmc.c |   48 
 -
  drivers/mmc/host/dw_mmc.h |1 +
  2 files changed, 48 insertions(+), 1 deletion(-)

 diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
 index 32dd81d..1d77431 100644
 --- a/drivers/mmc/host/dw_mmc.c
 +++ b/drivers/mmc/host/dw_mmc.c
 @@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct 
 dw_mci *host)
 host-sg = NULL;
 }

 -   return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
 +   /*
 +* The recommended method for resetting is to always reset the
 +* controller and the fifo, but differs slightly depending on 
 the mode.
 +* Note that this doesn't handle the generic DMA (not IDMAC) 
 case.
 +*/

 not IDMAC is confused..


 The documentation describes three different possible modes.  There's a
 mode that doesn't use IDMAC but still does DMA.  But as far as I can
 tell this driver doesn't support that way.  We can just remove that
 wording if it's confusing.

 How did it know whether dma is generic DMA or not?


 That's a good question.  I wasn't sure whether the driver supported it
 or not.  It looks like it definitely supports IDMAC through the
 CONFIG_MMC_DW_IDMAC flag, but I wasn't sure if it was supported the
 generic dma.  Maybe if CONFIG_MMC_DW_IDMAC isn't specified but
 host-dma_ops is not NULL then we are using the generic dma mode.


Doug mentioned that James Hogan might have an answer.  James, are
there Imgtec SoCs which use dw_mmc and use DMA but don't use the
IDMAC?  If so, we can add that support into this reset procedure
patch.


 +   if (dw_mci_ctrl_reset(host, SDMMC_CTRL_RESET | 
 SDMMC_CTRL_FIFO_RESET)) {
 +   unsigned long timeout = jiffies + msecs_to_jiffies(500);
 +   u32 status, rint;
 +
 +   /* if using dma we wait for dma_req to clear */
 +   if (host-using_dma) {
 +   do {
 +   status = mci_readl(host, STATUS);
 +   if (!(status  SDMMC_STATUS_DMA_REQ))
 +   break;
 +   cpu_relax();
 +   } while (time_before(jiffies, timeout));
 +
 +   if (status  SDMMC_STATUS_DMA_REQ)
 +   dev_err(host-dev,
 +   %s: Timeout waiting for 
 dma_req to 
 +   clear during reset, __func__);
 +
 +   /* when using DMA next we reset the fifo again 
 */
 +   dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
 +   }
 +   /*
 +* In all cases we clear the RAWINTS register to clear 
 any
 +* interrupts.
 +*/
 +   rint = mci_readl(host, RINTSTS);
 +   rint = rint  (~mci_readl(host, MINTSTS));
 you use the status or temp instead of rint. (you can reuse the variable.)
 And can use status = ~mci_readl(host,MINTSTS);


 Just clear the RINTSTS register? why do you add these?


 This will look at what is not masked, and only clear those bits.
 Well, i known if clear the RINTSTS register,
 recommended to use 0xfff and set the value for interrupt.

 Can be used 0xfff?


 Yeah we probably can.  We just lose information about interrupts that
 were masked, but maybe we just don't care about any of them anyway, so
 it doesn't matter.

 Best Regards,
 Jaehoon Chung


 +   if (rint)
 +   mci_writel(host, RINTSTS, rint);
 +
 +   } else
 +   dev_err(host-dev, %s: Reset bits didn't clear, 
 __func__);

 Just display the error log? I didn't understand this.
 If you displayed the error log, then it needs to return the error value

Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure

2014-05-12 Thread Sonny Rao

On Mon, May 12, 2014 at 2:44 PM, Sonny Rao sonny...@chromium.org wrote:
 On Fri, May 9, 2014 at 8:36 PM, Sonny Rao sonny...@chromium.org wrote:
 On Fri, May 9, 2014 at 12:32 AM, Jaehoon Chung jh80.ch...@samsung.com 
 wrote:
 Hi, Sonny.

 You can discard the my previous some comment.
 As you mentioned, this reset sequence is recommended at Synopsys TRM.

 Add the minor question.

 On 05/09/2014 01:27 PM, Jaehoon Chung wrote:
 Hi, Sonny.

 I have checked the Synopsys TRM..

 On 05/09/2014 10:34 AM, Sonny Rao wrote:
 On Thu, May 8, 2014 at 6:15 PM, Jaehoon Chung jh80.ch...@samsung.com 
 wrote:
 On 05/08/2014 06:40 PM, Yuvaraj Kumar wrote:
 Any comments on this patch?

 On Wed, Mar 26, 2014 at 5:16 PM, Yuvaraj Kumar C D 
 yuvaraj...@gmail.com wrote:
 From: Sonny Rao sonny...@chromium.org

 This patch changes the fifo reset code to follow the reset procedure
 outlined in the documentation of Synopsys  Mobile storage host databook
 7.2.13.
 Without this patch, we could able to see eMMC was not detected after
 multiple reboots due to driver hangs while eMMC tuning for HS200.

 Signed-off-by: Sonny Rao sonny...@chromium.org
 Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.org
 ---
  drivers/mmc/host/dw_mmc.c |   48 
 -
  drivers/mmc/host/dw_mmc.h |1 +
  2 files changed, 48 insertions(+), 1 deletion(-)

 diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
 index 32dd81d..1d77431 100644
 --- a/drivers/mmc/host/dw_mmc.c
 +++ b/drivers/mmc/host/dw_mmc.c
 @@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct 
 dw_mci *host)
 host-sg = NULL;
 }

 -   return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
 +   /*
 +* The recommended method for resetting is to always reset the
 +* controller and the fifo, but differs slightly depending on 
 the mode.
 +* Note that this doesn't handle the generic DMA (not IDMAC) 
 case.
 +*/

 not IDMAC is confused..


 The documentation describes three different possible modes.  There's a
 mode that doesn't use IDMAC but still does DMA.  But as far as I can
 tell this driver doesn't support that way.  We can just remove that
 wording if it's confusing.

 How did it know whether dma is generic DMA or not?


 That's a good question.  I wasn't sure whether the driver supported it
 or not.  It looks like it definitely supports IDMAC through the
 CONFIG_MMC_DW_IDMAC flag, but I wasn't sure if it was supported the
 generic dma.  Maybe if CONFIG_MMC_DW_IDMAC isn't specified but
 host-dma_ops is not NULL then we are using the generic dma mode.


 Doug mentioned that James Hogan might have an answer.  James, are
 there Imgtec SoCs which use dw_mmc and use DMA but don't use the
 IDMAC?  If so, we can add that support into this reset procedure
 patch.


In any case, whether on not anybody is using it, it looks like it's
not that hard to support this mode for reset (I was just lazy the
first time), we just need to add a flag to our reset.  I've made that
change and cleaned it up a little bit more and I'll resend this patch.


 +   if (dw_mci_ctrl_reset(host, SDMMC_CTRL_RESET | 
 SDMMC_CTRL_FIFO_RESET)) {
 +   unsigned long timeout = jiffies + 
 msecs_to_jiffies(500);
 +   u32 status, rint;
 +
 +   /* if using dma we wait for dma_req to clear */
 +   if (host-using_dma) {
 +   do {
 +   status = mci_readl(host, STATUS);
 +   if (!(status  SDMMC_STATUS_DMA_REQ))
 +   break;
 +   cpu_relax();
 +   } while (time_before(jiffies, timeout));
 +
 +   if (status  SDMMC_STATUS_DMA_REQ)
 +   dev_err(host-dev,
 +   %s: Timeout waiting for 
 dma_req to 
 +   clear during reset, 
 __func__);
 +
 +   /* when using DMA next we reset the fifo again 
 */
 +   dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
 +   }
 +   /*
 +* In all cases we clear the RAWINTS register to clear 
 any
 +* interrupts.
 +*/
 +   rint = mci_readl(host, RINTSTS);
 +   rint = rint  (~mci_readl(host, MINTSTS));
 you use the status or temp instead of rint. (you can reuse the variable.)
 And can use status = ~mci_readl(host,MINTSTS);


 Just clear the RINTSTS register? why do you add these?


 This will look at what is not masked, and only clear those bits.
 Well, i known if clear the RINTSTS register,
 recommended to use 0xfff and set the value for interrupt.

 Can be used 0xfff?


 Yeah we probably can.  We just lose information about interrupts that
 were masked, but maybe we just don't care about any of them anyway, so
 it doesn't

[PATCHv2] mmc: dw_mmc: change to use recommended reset procedure

2014-05-12 Thread Sonny Rao

This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys  Mobile storage host databook
7.2.13.

v2: Add Generic DMA support
per the documentation, move interrupt clear before wait
make the test for DMA host-use_dma rather than host-using_dma
add proper return values (although it appears no caller checks)

Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
---
 drivers/mmc/host/dw_mmc.c | 55 ++-
 drivers/mmc/host/dw_mmc.h |  1 +
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 55cd110..aff57e1 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2325,6 +2325,7 @@ static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 
reset)
 
 static inline bool dw_mci_fifo_reset(struct dw_mci *host)
 {
+   u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
/*
 * Reseting generates a block interrupt, hence setting
 * the scatter-gather pointer to NULL.
@@ -2334,7 +2335,59 @@ static inline bool dw_mci_fifo_reset(struct dw_mci *host)
host-sg = NULL;
}
 
-   return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+   /*
+* The recommended method for resetting is to always reset the
+* controller and the fifo, but differs slightly depending on the mode.
+* The Generic DMA mode (non IDMAC) also needs to reset DMA where IDMAC
+* mode resets IDMAC at the end.
+*
+*/
+#ifndef CONFIG_MMC_DW_IDMAC
+   if (host-use_dma)
+   flags |= SDMMC_CTRL_DMA_RESET;
+#endif
+   if (dw_mci_ctrl_reset(host, flags)) {
+   /*
+* In all cases we clear the RAWINTS register to clear any
+* interrupts.
+*/
+   mci_writel(host, RINTSTS, 0x);
+
+   /* if using dma we wait for dma_req to clear */
+   if (host-use_dma) {
+   unsigned long timeout = jiffies + msecs_to_jiffies(500);
+   u32 status;
+   do {
+   status = mci_readl(host, STATUS);
+   if (!(status  SDMMC_STATUS_DMA_REQ))
+   break;
+   cpu_relax();
+   } while (time_before(jiffies, timeout));
+
+   if (status  SDMMC_STATUS_DMA_REQ) {
+   dev_err(host-dev,
+   %s: Timeout waiting for dma_req to 
+   clear during reset, __func__);
+   return false;
+   }
+
+   /* when using DMA next we reset the fifo again */
+   dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+   }
+   } else {
+   dev_err(host-dev, %s: Reset bits didn't clear, __func__);
+   return false;
+   }
+
+#ifdef CONFIG_MMC_DW_IDMAC
+   /* It is also recommended that we reset and reprogram idmac */
+   dw_mci_idmac_reset(host);
+#endif
+
+   /* After a CTRL reset we need to have CIU set clock registers  */
+   mci_send_cmd(host-cur_slot, SDMMC_CMD_UPD_CLK, 0);
+
+   return true;
 }
 
 static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 6bf24ab..2505804 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -129,6 +129,7 @@
 #define SDMMC_CMD_INDX(n)  ((n)  0x1F)
 /* Status register defines */
 #define SDMMC_GET_FCNT(x)  (((x)17)  0x1FFF)
+#define SDMMC_STATUS_DMA_REQ   BIT(31)
 /* FIFOTH register defines */
 #define SDMMC_SET_FIFOTH(m, r, t)  (((m)  0x7)  28 | \
 ((r)  0xFFF)  16 | \
-- 
1.9.1.423.g4596e3a

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc 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: dw_mmc: change to use recommended reset procedure

2014-05-09 Thread Sonny Rao

On Fri, May 9, 2014 at 12:32 AM, Jaehoon Chung jh80.ch...@samsung.com wrote:
 Hi, Sonny.

 You can discard the my previous some comment.
 As you mentioned, this reset sequence is recommended at Synopsys TRM.

 Add the minor question.

 On 05/09/2014 01:27 PM, Jaehoon Chung wrote:
 Hi, Sonny.

 I have checked the Synopsys TRM..

 On 05/09/2014 10:34 AM, Sonny Rao wrote:
 On Thu, May 8, 2014 at 6:15 PM, Jaehoon Chung jh80.ch...@samsung.com 
 wrote:
 On 05/08/2014 06:40 PM, Yuvaraj Kumar wrote:
 Any comments on this patch?

 On Wed, Mar 26, 2014 at 5:16 PM, Yuvaraj Kumar C D yuvaraj...@gmail.com 
 wrote:
 From: Sonny Rao sonny...@chromium.org

 This patch changes the fifo reset code to follow the reset procedure
 outlined in the documentation of Synopsys  Mobile storage host databook
 7.2.13.
 Without this patch, we could able to see eMMC was not detected after
 multiple reboots due to driver hangs while eMMC tuning for HS200.

 Signed-off-by: Sonny Rao sonny...@chromium.org
 Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.org
 ---
  drivers/mmc/host/dw_mmc.c |   48 
 -
  drivers/mmc/host/dw_mmc.h |1 +
  2 files changed, 48 insertions(+), 1 deletion(-)

 diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
 index 32dd81d..1d77431 100644
 --- a/drivers/mmc/host/dw_mmc.c
 +++ b/drivers/mmc/host/dw_mmc.c
 @@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct 
 dw_mci *host)
 host-sg = NULL;
 }

 -   return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
 +   /*
 +* The recommended method for resetting is to always reset the
 +* controller and the fifo, but differs slightly depending on 
 the mode.
 +* Note that this doesn't handle the generic DMA (not IDMAC) 
 case.
 +*/

 not IDMAC is confused..


 The documentation describes three different possible modes.  There's a
 mode that doesn't use IDMAC but still does DMA.  But as far as I can
 tell this driver doesn't support that way.  We can just remove that
 wording if it's confusing.

 How did it know whether dma is generic DMA or not?


That's a good question.  I wasn't sure whether the driver supported it
or not.  It looks like it definitely supports IDMAC through the
CONFIG_MMC_DW_IDMAC flag, but I wasn't sure if it was supported the
generic dma.  Maybe if CONFIG_MMC_DW_IDMAC isn't specified but
host-dma_ops is not NULL then we are using the generic dma mode.


 +   if (dw_mci_ctrl_reset(host, SDMMC_CTRL_RESET | 
 SDMMC_CTRL_FIFO_RESET)) {
 +   unsigned long timeout = jiffies + msecs_to_jiffies(500);
 +   u32 status, rint;
 +
 +   /* if using dma we wait for dma_req to clear */
 +   if (host-using_dma) {
 +   do {
 +   status = mci_readl(host, STATUS);
 +   if (!(status  SDMMC_STATUS_DMA_REQ))
 +   break;
 +   cpu_relax();
 +   } while (time_before(jiffies, timeout));
 +
 +   if (status  SDMMC_STATUS_DMA_REQ)
 +   dev_err(host-dev,
 +   %s: Timeout waiting for dma_req 
 to 
 +   clear during reset, __func__);
 +
 +   /* when using DMA next we reset the fifo again */
 +   dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
 +   }
 +   /*
 +* In all cases we clear the RAWINTS register to clear 
 any
 +* interrupts.
 +*/
 +   rint = mci_readl(host, RINTSTS);
 +   rint = rint  (~mci_readl(host, MINTSTS));
 you use the status or temp instead of rint. (you can reuse the variable.)
 And can use status = ~mci_readl(host,MINTSTS);


 Just clear the RINTSTS register? why do you add these?


 This will look at what is not masked, and only clear those bits.
 Well, i known if clear the RINTSTS register,
 recommended to use 0xfff and set the value for interrupt.

 Can be used 0xfff?


Yeah we probably can.  We just lose information about interrupts that
were masked, but maybe we just don't care about any of them anyway, so
it doesn't matter.

 Best Regards,
 Jaehoon Chung


 +   if (rint)
 +   mci_writel(host, RINTSTS, rint);
 +
 +   } else
 +   dev_err(host-dev, %s: Reset bits didn't clear, 
 __func__);

 Just display the error log? I didn't understand this.
 If you displayed the error log, then it needs to return the error value.

 +
 + #ifdef CONFIG_MMC_DW_IDMAC
 +   /* It is also recommended that we reset and reprogram idmac */
 +   dw_mci_idmac_reset(host);
 + #endif
 +
 +   /* After a CTRL reset we need to have CIU set clock registers  */
 +   mci_send_cmd(host-cur_slot, SDMMC_CMD_UPD_CLK, 0);

 Well

Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure

2014-05-08 Thread Sonny Rao

On Thu, May 8, 2014 at 6:15 PM, Jaehoon Chung jh80.ch...@samsung.com wrote:
 On 05/08/2014 06:40 PM, Yuvaraj Kumar wrote:
 Any comments on this patch?

 On Wed, Mar 26, 2014 at 5:16 PM, Yuvaraj Kumar C D yuvaraj...@gmail.com 
 wrote:
 From: Sonny Rao sonny...@chromium.org

 This patch changes the fifo reset code to follow the reset procedure
 outlined in the documentation of Synopsys  Mobile storage host databook
 7.2.13.
 Without this patch, we could able to see eMMC was not detected after
 multiple reboots due to driver hangs while eMMC tuning for HS200.

 Signed-off-by: Sonny Rao sonny...@chromium.org
 Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.org
 ---
  drivers/mmc/host/dw_mmc.c |   48 
 -
  drivers/mmc/host/dw_mmc.h |1 +
  2 files changed, 48 insertions(+), 1 deletion(-)

 diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
 index 32dd81d..1d77431 100644
 --- a/drivers/mmc/host/dw_mmc.c
 +++ b/drivers/mmc/host/dw_mmc.c
 @@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct dw_mci 
 *host)
 host-sg = NULL;
 }

 -   return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
 +   /*
 +* The recommended method for resetting is to always reset the
 +* controller and the fifo, but differs slightly depending on the 
 mode.
 +* Note that this doesn't handle the generic DMA (not IDMAC) case.
 +*/

 not IDMAC is confused..


The documentation describes three different possible modes.  There's a
mode that doesn't use IDMAC but still does DMA.  But as far as I can
tell this driver doesn't support that way.  We can just remove that
wording if it's confusing.

 +   if (dw_mci_ctrl_reset(host, SDMMC_CTRL_RESET | 
 SDMMC_CTRL_FIFO_RESET)) {
 +   unsigned long timeout = jiffies + msecs_to_jiffies(500);
 +   u32 status, rint;
 +
 +   /* if using dma we wait for dma_req to clear */
 +   if (host-using_dma) {
 +   do {
 +   status = mci_readl(host, STATUS);
 +   if (!(status  SDMMC_STATUS_DMA_REQ))
 +   break;
 +   cpu_relax();
 +   } while (time_before(jiffies, timeout));
 +
 +   if (status  SDMMC_STATUS_DMA_REQ)
 +   dev_err(host-dev,
 +   %s: Timeout waiting for dma_req to 
 
 +   clear during reset, __func__);
 +
 +   /* when using DMA next we reset the fifo again */
 +   dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
 +   }
 +   /*
 +* In all cases we clear the RAWINTS register to clear any
 +* interrupts.
 +*/
 +   rint = mci_readl(host, RINTSTS);
 +   rint = rint  (~mci_readl(host, MINTSTS));

 Just clear the RINTSTS register? why do you add these?


This will look at what is not masked, and only clear those bits.

 +   if (rint)
 +   mci_writel(host, RINTSTS, rint);
 +
 +   } else
 +   dev_err(host-dev, %s: Reset bits didn't clear, __func__);

 Just display the error log? I didn't understand this.
 If you displayed the error log, then it needs to return the error value.

 +
 + #ifdef CONFIG_MMC_DW_IDMAC
 +   /* It is also recommended that we reset and reprogram idmac */
 +   dw_mci_idmac_reset(host);
 + #endif
 +
 +   /* After a CTRL reset we need to have CIU set clock registers  */
 +   mci_send_cmd(host-cur_slot, SDMMC_CMD_UPD_CLK, 0);
 +
 +   return true;
  }

  static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
 diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
 index 738fa24..037e47a 100644
 --- a/drivers/mmc/host/dw_mmc.h
 +++ b/drivers/mmc/host/dw_mmc.h
 @@ -129,6 +129,7 @@
  #define SDMMC_CMD_INDX(n)  ((n)  0x1F)
  /* Status register defines */
  #define SDMMC_GET_FCNT(x)  (((x)17)  0x1FFF)
 +#define SDMMC_STATUS_DMA_REQ   BIT(31)
  /* FIFOTH register defines */
  #define SDMMC_SET_FIFOTH(m, r, t)  (((m)  0x7)  28 | \
  ((r)  0xFFF)  16 | \
 --
 1.7.10.4



--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc 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: dw_mmc: Make sure we don't get stuck when we get an error

2014-05-08 Thread Sonny Rao

On Thu, May 8, 2014 at 2:42 AM, Yuvaraj Kumar yuvaraj...@gmail.com wrote:
 Any comments on this patch?


I'll just add that without this fix, running the tuning loop for UHS
modes is not reliable on dw_mmc because errors will happen and you
will eventually hit this race and hang.  This can happen any time
there is tuning like during boot or during resume from suspend.

 On Thu, Mar 27, 2014 at 11:48 AM, Yuvaraj Kumar C D
 yuvaraj...@gmail.com wrote:
 From: Doug Anderson diand...@chromium.org

 If we happened to get a data error at just the wrong time the dw_mmc
 driver could get into a state where it would never complete its
 request.  That would leave the caller just hanging there.

 We fix this two ways and both of the two fixes on their own appear to
 fix the problems we've seen:

 1. Fix a race in the tasklet where the interrupt setting the data
error happens _just after_ we check for it, then we get a
EVENT_XFER_COMPLETE.  We fix this by repeating a bit of code.
 2. Fix it so that if we detect that we've got an error in the data
busy state and we're not going to do anything else we end the
request and unblock anyone waiting.

 Signed-off-by: Doug Anderson diand...@chromium.org
 Signed-off-by: Yuvaraj Kumar C D yuvaraj...@gmail.com
 ---
  drivers/mmc/host/dw_mmc.c |   47 
 +
  1 file changed, 47 insertions(+)

 diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
 index 1d77431..4c589f1 100644
 --- a/drivers/mmc/host/dw_mmc.c
 +++ b/drivers/mmc/host/dw_mmc.c
 @@ -1300,6 +1300,14 @@ static void dw_mci_tasklet_func(unsigned long priv)
 /* fall through */

 case STATE_SENDING_DATA:
 +   /*
 +* We could get a data error and never a transfer
 +* complete so we'd better check for it here.
 +*
 +* Note that we don't really care if we also got a
 +* transfer complete; stopping the DMA and sending an
 +* abort won't hurt.
 +*/
 if (test_and_clear_bit(EVENT_DATA_ERROR,
host-pending_events)) {
 dw_mci_stop_dma(host);
 @@ -1313,7 +1321,29 @@ static void dw_mci_tasklet_func(unsigned long priv)
 break;

 set_bit(EVENT_XFER_COMPLETE, 
 host-completed_events);
 +
 +   /*
 +* Handle an EVENT_DATA_ERROR that might have shown 
 up
 +* before the transfer completed.  This might not 
 have
 +* been caught by the check above because the 
 interrupt
 +* could have gone off between the previous check and
 +* the check for transfer complete.
 +*
 +* Technically this ought not be needed assuming we
 +* get a DATA_COMPLETE eventually (we'll notice the
 +* error and end the request), but it shouldn't hurt.
 +*
 +* This has the advantage of sending the stop 
 command.
 +*/
 +   if (test_and_clear_bit(EVENT_DATA_ERROR,
 +  host-pending_events)) {
 +   dw_mci_stop_dma(host);
 +   send_stop_abort(host, data);
 +   state = STATE_DATA_ERROR;
 +   break;
 +   }
 prev_state = state = STATE_DATA_BUSY;
 +
 /* fall through */

 case STATE_DATA_BUSY:
 @@ -1336,6 +1366,23 @@ static void dw_mci_tasklet_func(unsigned long priv)
 /* stop command for open-ended transfer*/
 if (data-stop)
 send_stop_abort(host, data);
 +   } else {
 +   /*
 +* If we don't have a command complete now 
 we'll
 +* never get one since we just reset 
 everything;
 +* better end the request.
 +*
 +* If we do have a command complete we'll 
 fall
 +* through to the SENDING_STOP command and
 +* everything will be peachy keen.
 +*
 +* TODO: I guess we shouldn't send a stop?
 +*/
 +   if (!test_bit(EVENT_CMD_COMPLETE,
 +

Re: [PATCH] ARM: dts: Fix mmc node on exynos5250 snow board

2013-12-05 Thread Sonny Rao

On Thu, Dec 5, 2013 at 2:06 AM, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote:
 Commits 64c138a (ARM: dts: Move fifo-depth property from exynos5250
 board dts) and 0c3de788 (ARM: dts: change status property of dwmmc
 nodes for exynos5250) missed out handling the exynos5250 snow dts file.
 Delete the fifo-depth property and enable the mmc node in the snow dts
 file.


Since this is really fixing two different issues, even though they
both affect dw_mmc, would you mind splitting this up into two
different commits?

 Signed-off-by : Abhilash Kesavan a.kesa...@samsung.com

 Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
 ---
  arch/arm/boot/dts/cros5250-common.dtsi |7 ---
  arch/arm/boot/dts/exynos5250-snow.dts  |9 +
  2 files changed, 9 insertions(+), 7 deletions(-)

 diff --git a/arch/arm/boot/dts/cros5250-common.dtsi 
 b/arch/arm/boot/dts/cros5250-common.dtsi
 index 6470536..2dd70e9 100644
 --- a/arch/arm/boot/dts/cros5250-common.dtsi
 +++ b/arch/arm/boot/dts/cros5250-common.dtsi
 @@ -233,7 +233,6 @@
 num-slots = 1;
 supports-highspeed;
 broken-cd;
 -   fifo-depth = 0x80;

Why are you deleting fifo-depth?

 card-detect-delay = 200;
 samsung,dw-mshc-ciu-div = 3;
 samsung,dw-mshc-sdr-timing = 2 3;
 @@ -247,14 +246,9 @@
 };
 };

 -   mmc@1221 {
 -   status = disabled;
 -   };
 -
 mmc@1222 {
 num-slots = 1;
 supports-highspeed;
 -   fifo-depth = 0x80;
 card-detect-delay = 200;
 samsung,dw-mshc-ciu-div = 3;
 samsung,dw-mshc-sdr-timing = 2 3;
 @@ -273,7 +267,6 @@
 num-slots = 1;
 supports-highspeed;
 broken-cd;
 -   fifo-depth = 0x80;
 card-detect-delay = 200;
 samsung,dw-mshc-ciu-div = 3;
 samsung,dw-mshc-sdr-timing = 2 3;
 diff --git a/arch/arm/boot/dts/exynos5250-snow.dts 
 b/arch/arm/boot/dts/exynos5250-snow.dts
 index a9395c4..67484d1 100644
 --- a/arch/arm/boot/dts/exynos5250-snow.dts
 +++ b/arch/arm/boot/dts/exynos5250-snow.dts
 @@ -171,11 +171,20 @@
 };
 };

 +   mmc@1220 {
 +   status = okay;
 +   };
 +
 +   mmc@1222 {
 +   status = okay;
 +   };
 +
 /*
  * On Snow we've got SIP WiFi and so can keep drive strengths low to
  * reduce EMI.
  */
 mmc@1223 {
 +   status = okay;
 slot@0 {
 pinctrl-names = default;
 pinctrl-0 = sd3_clk sd3_cmd sd3_bus4;
 --
 1.7.9.5

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

Re: [PATCH] clk: samsung: Fix PLL35XX lock time

2013-10-15 Thread Sonny Rao

On Mon, Oct 14, 2013 at 9:08 PM, Doug Anderson diand...@chromium.org wrote:
 Tomasz,

 On Fri, Oct 11, 2013 at 7:06 PM, Tomasz Figa t.f...@samsung.com wrote:
 Well, it's some kind of difference indeed. However, how often can
 a frequency transition happen?

 I believe that ondemand allows minimum sampling period of
 100 * transition latency, so even without considering the PLL lock time,
 we would have at most a single delay of ~400 us every ~40 ms, during
 which remaining CPUs are operating normally, because of switching to
 MPLL temporarily.

 A bit more interesting case would be the Android interactive governor,
 which scales up the CPU on any wake-up event, but I don't believe any
 user would notice the extra 17,5 us from touching the screen to seeing
 a menu animation, for example.

 OK.  You've clearly looked at this more than I.  :)  I was merely
 recalling conversations that others had over a year ago and
 remembering some of our engineers being concerned about latencies even
 at this level, but I'm not sure if they had hard data.  I added Sonny
 to see if he might remember more, though I don't think he was the one
 pushing for optimizations in the past.


 Note that we actually are using the Interactive governor in our
 systems, and it is the interactive time to spin up on user input
 that I'm most worried about.  ...but you're right that I think the
 user perceivable values are in the 10s of milliseconds and not in the
 10s of microseconds.


Yeah, we're using interactive, and we're typically running on the
slowest clock frequency when a user uses an input device that causes a
boost to happen, and I think the initial measurements (last year) of
how long it took to complete just the voltage transition on Snow were
something like 1 millisecond mostly because of how udelay worked by
using a fixed number of loops.  That seemed pretty long but wasn't
considered terrible because after we got up to a higher speed it
wouldn't take as long to make further transitions, and I also didn't
expect us to make that transition often enough that it would be a
significant impact on CPU usage, and 1ms probably isn't quite enough
for a user to notice.



 Please correct me if I'm missing something here.

 Anyway, when it's something as simple as passing in a parameter to get
 it right, it seems worth doing.

 Sure, any patches improving things are welcome, I was just wondering
 whether the improvement will be of any significance. Looking forward
 to respective patches, hoping that they will not complicate the code
 too much (although we already have a struct for PLL parameters, so they
 should not).

 I'd still love to see the change land that allows this to be right.
 17.5us may not matter much, but with the PLL parameter struct it
 should be pretty easy / clean.  ...and it also allows us to map the
 number back the user manual, which can make it easier to understand
 (there's currently nothing documenting why the number is 270, not 250
 or 200).

 -Doug
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc 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 3/3] mmc: dw_mmc: Dont cut off vqmmc and vmmc

Re: [PATCH V2 3/3] mmc: dw_mmc: Dont cut off vqmmc and vmmc

[PATCH] mmc: dw_mmc: change to use recommended reset procedure

[PATCHv6] mmc: dw_mmc: change to use recommended reset procedure

Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure

[PATCHv5] mmc: dw_mmc: change to use recommended reset procedure

Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure

Using s5p-sss hw crypto causes ipsec to break

Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure

[PATCH] mmc: dw_mmc: change to use recommended reset procedure

[PATCH] mmc: dw_mmc: change to use recommended reset procedure

Re: [PATCHv2] mmc: dw_mmc: change to use recommended reset procedure

Re: [PATCHv2] mmc: dw_mmc: change to use recommended reset procedure

Re: [PATCH] arm: dts: exynos5: Remove multi core timer

Re: [PATCHv2] mmc: dw_mmc: change to use recommended reset procedure

Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure

Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure

Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure

[PATCHv2] mmc: dw_mmc: change to use recommended reset procedure

Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure

Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure

Re: [PATCH] mmc: dw_mmc: Make sure we don't get stuck when we get an error

Re: [PATCH] ARM: dts: Fix mmc node on exynos5250 snow board

Re: [PATCH] clk: samsung: Fix PLL35XX lock time

24 matches

Site Navigation

Mail list logo

Footer information