[PATCH 00/10] Assorted MMC / OMAP HSMMC patches

[PATCH 00/10] Assorted MMC / OMAP HSMMC patches

[Venkatraman S]

Essentially, a lot of cleanups leading up to adding a new
feature for OMAP HSMMC. The idea is to convert to the use
of software timer instead of IP timer for timekeeping, due
to the limitations of the counting range of the IP timer.

Also added myself as OMAP HSMMC maintainer.

Patch 9/10 is in draft state and needs more testing.

These patches are also available at
  git://github.com/svenkatr/linux.git my/mmc/3.6/hrtimer_updates

Venkatraman S (10):
  mmc: core: Add TRANsfer state to non-HPI state
  mmc: debugfs: Print ext_csd in ascending order
  mmc: omap: remove unused variables and includes
  mmc: omap: fix mmc_omap_report_irq to use dev_dbg macros
  mmc: omap_hsmmc: remove unused vars and includes
  mmc: omap_hsmmc: remove access to SYSCONFIG register
  mmc: omap_hsmmc: consolidate flush posted writes for HSMMC IRQs
  mmc: omap_hsmmc: consolidate error report handling of HSMMC IRQ
  mmc: omap_hsmmc: convert from IP timer to hrtimer
  mmc: omap_hsmmc: Move to Maintained state in MAINTAINERS

 MAINTAINERS                   |   4 +-
 drivers/mmc/core/core.c       |   3 +-
 drivers/mmc/core/debugfs.c    |   2 +-
 drivers/mmc/host/omap.c       |  39 ++++----
 drivers/mmc/host/omap_hsmmc.c | 212 ++++++++++++++++--------------------------
 5 files changed, 103 insertions(+), 157 deletions(-)

-- 
1.7.11.1.25.g0e18bef

[PATCH 01/10] mmc: core: Add TRANsfer state to non-HPI state

[Venkatraman S]

HPI can be issued only in programming state to bring the card to
transfer state. If the card is already in transfer state, doing
a HPI is redundant.
Fix this by adding transfer state to the list of exceptions to
doing HPI and return without error.

Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
 drivers/mmc/core/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 8ac5246..835c9f0 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -424,8 +424,9 @@ int mmc_interrupt_hpi(struct mmc_card *card)
 	case R1_STATE_IDLE:
 	case R1_STATE_READY:
 	case R1_STATE_STBY:
+	case R1_STATE_TRAN:
 		/*
-		 * In idle states, HPI is not needed and the caller
+		 * In idle and transfer states, HPI is not needed and the caller
 		 * can issue the next intended command immediately
 		 */
 		goto out;
-- 
1.7.11.1.25.g0e18bef

[PATCH 02/10] mmc: debugfs: Print ext_csd in ascending order

[Venkatraman S]

ext_csd exported through debugfs is printed in reverse order (from
byte 511 to 0), which causes confusion.
Fix the for loop to print ext_csd in natural order.

Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
 drivers/mmc/core/debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
index 9ab5b17..d96c643 100644
--- a/drivers/mmc/core/debugfs.c
+++ b/drivers/mmc/core/debugfs.c
@@ -281,7 +281,7 @@ static int mmc_ext_csd_open(struct inode *inode, struct file *filp)
 	if (err)
 		goto out_free;
 
-	for (i = 511; i >= 0; i--)
+	for (i = 0; i < 512; i++)
 		n += sprintf(buf + n, "%02x", ext_csd[i]);
 	n += sprintf(buf + n, "\n");
 	BUG_ON(n != EXT_CSD_STR_LEN);
-- 
1.7.11.1.25.g0e18bef

[PATCH 03/10] mmc: omap: remove unused variables and includes

[Venkatraman S]

Get rid of some unnecessary includes in the driver and
a few unused variables.

Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
 drivers/mmc/host/omap.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c
index 50e08f0..0ec4e55 100644
--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -27,18 +27,10 @@
 #include <linux/mmc/card.h>
 #include <linux/clk.h>
 #include <linux/scatterlist.h>
-#include <linux/i2c/tps65010.h>
 #include <linux/slab.h>
 
-#include <asm/io.h>
-#include <asm/irq.h>
-
-#include <plat/board.h>
 #include <plat/mmc.h>
-#include <asm/gpio.h>
 #include <plat/dma.h>
-#include <plat/mux.h>
-#include <plat/fpga.h>
 
 #define	OMAP_MMC_REG_CMD	0x00
 #define	OMAP_MMC_REG_ARGL	0x01
@@ -107,7 +99,6 @@ struct mmc_omap_slot {
 	u16			saved_con;
 	u16			bus_mode;
 	unsigned int		fclk_freq;
-	unsigned		powered:1;
 
 	struct tasklet_struct	cover_tasklet;
 	struct timer_list       cover_timer;
@@ -139,7 +130,6 @@ struct mmc_omap_host {
 	unsigned int		phys_base;
 	int			irq;
 	unsigned char		bus_mode;
-	unsigned char		hw_bus_mode;
 	unsigned int		reg_shift;
 
 	struct work_struct	cmd_abort_work;
-- 
1.7.11.1.25.g0e18bef

[PATCH 04/10] mmc: omap: fix mmc_omap_report_irq to use dev_dbg macros

[Venkatraman S]

The function mmc_omap_report_irq uses raw printks and the
actual output was disabled by a static variable. Make
the function use dev_vdbg macro and use it under the
standard CONFIG_MMC_DEBUG flag.

Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
 drivers/mmc/host/omap.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c
index 0ec4e55..614f63c 100644
--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -679,22 +679,29 @@ mmc_omap_xfer_data(struct mmc_omap_host *host, int write)
 	}
 }
 
-static inline void mmc_omap_report_irq(u16 status)
+#ifdef CONFIG_MMC_DEBUG
+static void mmc_omap_report_irq(struct mmc_omap_host *host, u16 status)
 {
 	static const char *mmc_omap_status_bits[] = {
 		"EOC", "CD", "CB", "BRS", "EOFB", "DTO", "DCRC", "CTO",
 		"CCRC", "CRW", "AF", "AE", "OCRB", "CIRQ", "CERR"
 	};
-	int i, c = 0;
+	int i;
+	char res[64], *buf = res;
+
+	buf += sprintf(buf, "MMC IRQ 0x%x:", status);
 
 	for (i = 0; i < ARRAY_SIZE(mmc_omap_status_bits); i++)
-		if (status & (1 << i)) {
-			if (c)
-				printk(" ");
-			printk("%s", mmc_omap_status_bits[i]);
-			c++;
-		}
+		if (status & (1 << i))
+			buf += sprintf(buf, " %s", mmc_omap_status_bits[i]);
+	dev_vdbg(mmc_dev(host->mmc), "%s\n", res);
+}
+#else
+static void mmc_omap_report_irq(struct mmc_omap_host *host, u16 status)
+{
 }
+#endif
+
 
 static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
 {
@@ -728,12 +735,10 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
 			cmd = host->cmd->opcode;
 		else
 			cmd = -1;
-#ifdef CONFIG_MMC_DEBUG
 		dev_dbg(mmc_dev(host->mmc), "MMC IRQ %04x (CMD %d): ",
 			status, cmd);
-		mmc_omap_report_irq(status);
-		printk("\n");
-#endif
+		mmc_omap_report_irq(host, status);
+
 		if (host->total_bytes_left) {
 			if ((status & OMAP_MMC_STAT_A_FULL) ||
 			    (status & OMAP_MMC_STAT_END_OF_DATA))
-- 
1.7.11.1.25.g0e18bef

[PATCH 05/10] mmc: omap_hsmmc: remove unused vars and includes

[Venkatraman S]

Some straight forward cleanup of unnecessary #include's
and host variables. Some of the verbose and redundant
debug messages are converted to use dev_vdbg.

Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
 drivers/mmc/host/omap_hsmmc.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index e180735..da4f5a7 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -35,7 +35,6 @@
 #include <linux/mmc/core.h>
 #include <linux/mmc/mmc.h>
 #include <linux/io.h>
-#include <linux/semaphore.h>
 #include <linux/gpio.h>
 #include <linux/regulator/consumer.h>
 #include <linux/pm_runtime.h>
@@ -162,8 +161,6 @@ struct omap_hsmmc_host {
 	unsigned int		dma_sg_idx;
 	unsigned char		bus_mode;
 	unsigned char		power_mode;
-	u32			*buffer;
-	u32			bytesleft;
 	int			suspended;
 	int			irq;
 	int			use_dma, dma_ch;
@@ -172,7 +169,6 @@ struct omap_hsmmc_host {
 	int			slot_id;
 	int			response_busy;
 	int			context_loss;
-	int			vdd;
 	int			protect_card;
 	int			reqs_blocked;
 	int			use_reg;
@@ -496,7 +492,7 @@ static void omap_hsmmc_set_clock(struct omap_hsmmc_host *host)
 	unsigned long regval;
 	unsigned long timeout;
 
-	dev_dbg(mmc_dev(host->mmc), "Set clock to %uHz\n", ios->clock);
+	dev_vdbg(mmc_dev(host->mmc), "Set clock to %uHz\n", ios->clock);
 
 	omap_hsmmc_stop_clock(host);
 
@@ -746,7 +742,7 @@ omap_hsmmc_start_command(struct omap_hsmmc_host *host, struct mmc_command *cmd,
 {
 	int cmdreg = 0, resptype = 0, cmdtype = 0;
 
-	dev_dbg(mmc_dev(host->mmc), "%s: CMD%d, argument 0x%08x\n",
+	dev_vdbg(mmc_dev(host->mmc), "%s: CMD%d, argument 0x%08x\n",
 		mmc_hostname(host->mmc), cmd->opcode, cmd->arg);
 	host->cmd = cmd;
 
@@ -935,7 +931,7 @@ static void omap_hsmmc_dbg_report_irq(struct omap_hsmmc_host *host, u32 status)
 			buf += len;
 		}
 
-	dev_dbg(mmc_dev(host->mmc), "%s\n", res);
+	dev_vdbg(mmc_dev(host->mmc), "%s\n", res);
 }
 #else
 static inline void omap_hsmmc_dbg_report_irq(struct omap_hsmmc_host *host,
@@ -997,7 +993,7 @@ static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
 	}
 
 	data = host->data;
-	dev_dbg(mmc_dev(host->mmc), "IRQ Status is %x\n", status);
+	dev_vdbg(mmc_dev(host->mmc), "IRQ Status is %x\n", status);
 
 	if (status & ERR) {
 		omap_hsmmc_dbg_report_irq(host, status);
@@ -1502,12 +1498,10 @@ static void omap_hsmmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		case MMC_POWER_OFF:
 			mmc_slot(host).set_power(host->dev, host->slot_id,
 						 0, 0);
-			host->vdd = 0;
 			break;
 		case MMC_POWER_UP:
 			mmc_slot(host).set_power(host->dev, host->slot_id,
 						 1, ios->vdd);
-			host->vdd = ios->vdd;
 			break;
 		case MMC_POWER_ON:
 			do_send_init_stream = 1;
-- 
1.7.11.1.25.g0e18bef

[PATCH 06/10] mmc: omap_hsmmc: remove access to SYSCONFIG register

[Venkatraman S]

SYSCONFIG register of HSMMC IP is managed by the omap hwmod
abstraction layer. Resetting the IP and configuring the correct
SYSCONFIG mode is centrally managed by hwmod.

Remove code which manipulates IP reset and SYSCONFIG directly in the
driver.

Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
 drivers/mmc/host/omap_hsmmc.c | 24 ++----------------------
 1 file changed, 2 insertions(+), 22 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index da4f5a7..4bc55ac 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -44,7 +44,6 @@
 #include <plat/cpu.h>
 
 /* OMAP HSMMC Host Controller Registers */
-#define OMAP_HSMMC_SYSCONFIG	0x0010
 #define OMAP_HSMMC_SYSSTATUS	0x0014
 #define OMAP_HSMMC_CON		0x002C
 #define OMAP_HSMMC_BLK		0x0104
@@ -576,21 +575,8 @@ static int omap_hsmmc_context_restore(struct omap_hsmmc_host *host)
 	if (host->context_loss == context_loss)
 		return 1;
 
-	/* Wait for hardware reset */
-	timeout = jiffies + msecs_to_jiffies(MMC_TIMEOUT_MS);
-	while ((OMAP_HSMMC_READ(host->base, SYSSTATUS) & RESETDONE) != RESETDONE
-		&& time_before(jiffies, timeout))
-		;
-
-	/* Do software reset */
-	OMAP_HSMMC_WRITE(host->base, SYSCONFIG, SOFTRESET);
-	timeout = jiffies + msecs_to_jiffies(MMC_TIMEOUT_MS);
-	while ((OMAP_HSMMC_READ(host->base, SYSSTATUS) & RESETDONE) != RESETDONE
-		&& time_before(jiffies, timeout))
-		;
-
-	OMAP_HSMMC_WRITE(host->base, SYSCONFIG,
-			OMAP_HSMMC_READ(host->base, SYSCONFIG) | AUTOIDLE);
+	if (!OMAP_HSMMC_READ(host->base, SYSSTATUS) & RESETDONE)
+		return 1;
 
 	if (host->pdata->controller_flags & OMAP_HSMMC_SUPPORTS_DUAL_VOLT) {
 		if (host->power_mode != MMC_POWER_OFF &&
@@ -1593,10 +1579,6 @@ static void omap_hsmmc_conf_bus_power(struct omap_hsmmc_host *host)
 	value = OMAP_HSMMC_READ(host->base, CAPA);
 	OMAP_HSMMC_WRITE(host->base, CAPA, value | capa);
 
-	/* Set the controller to AUTO IDLE mode */
-	value = OMAP_HSMMC_READ(host->base, SYSCONFIG);
-	OMAP_HSMMC_WRITE(host->base, SYSCONFIG, value | AUTOIDLE);
-
 	/* Set SD bus power bit */
 	set_sd_bus_power(host);
 }
@@ -1654,8 +1636,6 @@ static int omap_hsmmc_regs_show(struct seq_file *s, void *data)
 
 	pm_runtime_get_sync(host->dev);
 
-	seq_printf(s, "SYSCONFIG:\t0x%08x\n",
-			OMAP_HSMMC_READ(host->base, SYSCONFIG));
 	seq_printf(s, "CON:\t\t0x%08x\n",
 			OMAP_HSMMC_READ(host->base, CON));
 	seq_printf(s, "HCTL:\t\t0x%08x\n",
-- 
1.7.11.1.25.g0e18bef

Re: [PATCH 06/10] mmc: omap_hsmmc: remove access to SYSCONFIG register

[Shubhrajyoti Datta]

Hi Venkat,
Some doubts below.

On Sat, Aug 18, 2012 at 12:22 AM, Venkatraman S <svenkatr@ti.com> wrote:
> SYSCONFIG register of HSMMC IP is managed by the omap hwmod
> abstraction layer.

At init only right?


> Resetting the IP and configuring the correct
> SYSCONFIG mode is centrally managed by hwmod.
>
> Remove code which manipulates IP reset and SYSCONFIG directly in the
> driver.

I am not sure if mmc needs a reset.

However IMHO
In case it needs a reset

the hwmod way could be to use  omap_hwmod_reset
However I  fear it may be an issue with dt.
or
may be sysc could be restored something like [1]



if it doesnt need reset even the check for the SYSSTATUS could be
considered for removal.

[1] http://www.mail-archive.com/linux-mmc@vger.kernel.org/msg12041.html



>
> Signed-off-by: Venkatraman S <svenkatr@ti.com>
> ---
>  drivers/mmc/host/omap_hsmmc.c | 24 ++----------------------
>  1 file changed, 2 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index da4f5a7..4bc55ac 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -44,7 +44,6 @@
>  #include <plat/cpu.h>
>
>  /* OMAP HSMMC Host Controller Registers */
> -#define OMAP_HSMMC_SYSCONFIG   0x0010
>  #define OMAP_HSMMC_SYSSTATUS   0x0014
>  #define OMAP_HSMMC_CON         0x002C
>  #define OMAP_HSMMC_BLK         0x0104
> @@ -576,21 +575,8 @@ static int omap_hsmmc_context_restore(struct omap_hsmmc_host *host)
>         if (host->context_loss == context_loss)
>                 return 1;
>
> -       /* Wait for hardware reset */
> -       timeout = jiffies + msecs_to_jiffies(MMC_TIMEOUT_MS);
> -       while ((OMAP_HSMMC_READ(host->base, SYSSTATUS) & RESETDONE) != RESETDONE
> -               && time_before(jiffies, timeout))
> -               ;
> -
> -       /* Do software reset */
> -       OMAP_HSMMC_WRITE(host->base, SYSCONFIG, SOFTRESET);
> -       timeout = jiffies + msecs_to_jiffies(MMC_TIMEOUT_MS);
> -       while ((OMAP_HSMMC_READ(host->base, SYSSTATUS) & RESETDONE) != RESETDONE
> -               && time_before(jiffies, timeout))
> -               ;
> -
> -       OMAP_HSMMC_WRITE(host->base, SYSCONFIG,
> -                       OMAP_HSMMC_READ(host->base, SYSCONFIG) | AUTOIDLE);
> +       if (!OMAP_HSMMC_READ(host->base, SYSSTATUS) & RESETDONE)
> +               return 1;
Should this check be removed.

>
>         if (host->pdata->controller_flags & OMAP_HSMMC_SUPPORTS_DUAL_VOLT) {
>                 if (host->power_mode != MMC_POWER_OFF &&
> @@ -1593,10 +1579,6 @@ static void omap_hsmmc_conf_bus_power(struct omap_hsmmc_host *host)
>         value = OMAP_HSMMC_READ(host->base, CAPA);
>         OMAP_HSMMC_WRITE(host->base, CAPA, value | capa);
>
> -       /* Set the controller to AUTO IDLE mode */
> -       value = OMAP_HSMMC_READ(host->base, SYSCONFIG);
> -       OMAP_HSMMC_WRITE(host->base, SYSCONFIG, value | AUTOIDLE);
> -
>         /* Set SD bus power bit */
>         set_sd_bus_power(host);
>  }
> @@ -1654,8 +1636,6 @@ static int omap_hsmmc_regs_show(struct seq_file *s, void *data)
>
>         pm_runtime_get_sync(host->dev);
>
> -       seq_printf(s, "SYSCONFIG:\t0x%08x\n",
> -                       OMAP_HSMMC_READ(host->base, SYSCONFIG));
>         seq_printf(s, "CON:\t\t0x%08x\n",
>                         OMAP_HSMMC_READ(host->base, CON));
>         seq_printf(s, "HCTL:\t\t0x%08x\n",
> --
> 1.7.11.1.25.g0e18bef
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/10] mmc: omap_hsmmc: remove access to SYSCONFIG register

[S, Venkatraman]

On Tue, Aug 21, 2012 at 9:08 PM, Shubhrajyoti Datta
<omaplinuxkernel@gmail.com> wrote:
> Hi Venkat,
> Some doubts below.
>
> On Sat, Aug 18, 2012 at 12:22 AM, Venkatraman S <svenkatr@ti.com> wrote:
>> SYSCONFIG register of HSMMC IP is managed by the omap hwmod
>> abstraction layer.
>
> At init only right?
Yes.
>
>
>> Resetting the IP and configuring the correct
>> SYSCONFIG mode is centrally managed by hwmod.
>>
>> Remove code which manipulates IP reset and SYSCONFIG directly in the
>> driver.
>
> I am not sure if mmc needs a reset.
>
Actually it doesn't, as far as I know. That's why I propose it to be removed.

> However IMHO
> In case it needs a reset
>
> the hwmod way could be to use  omap_hwmod_reset
> However I  fear it may be an issue with dt.
> or
> may be sysc could be restored something like [1]
>
>
>
> if it doesnt need reset even the check for the SYSSTATUS could be
> considered for removal.
>
> [1] http://www.mail-archive.com/linux-mmc@vger.kernel.org/msg12041.html
>
>
>
>>
>> Signed-off-by: Venkatraman S <svenkatr@ti.com>
>> ---
>>  drivers/mmc/host/omap_hsmmc.c | 24 ++----------------------
>>  1 file changed, 2 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
>> index da4f5a7..4bc55ac 100644
>> --- a/drivers/mmc/host/omap_hsmmc.c
>> +++ b/drivers/mmc/host/omap_hsmmc.c
>> @@ -44,7 +44,6 @@
>>  #include <plat/cpu.h>
>>
>>  /* OMAP HSMMC Host Controller Registers */
>> -#define OMAP_HSMMC_SYSCONFIG   0x0010
>>  #define OMAP_HSMMC_SYSSTATUS   0x0014
>>  #define OMAP_HSMMC_CON         0x002C
>>  #define OMAP_HSMMC_BLK         0x0104
>> @@ -576,21 +575,8 @@ static int omap_hsmmc_context_restore(struct omap_hsmmc_host *host)
>>         if (host->context_loss == context_loss)
>>                 return 1;
>>
>> -       /* Wait for hardware reset */
>> -       timeout = jiffies + msecs_to_jiffies(MMC_TIMEOUT_MS);
>> -       while ((OMAP_HSMMC_READ(host->base, SYSSTATUS) & RESETDONE) != RESETDONE
>> -               && time_before(jiffies, timeout))
>> -               ;
>> -
>> -       /* Do software reset */
>> -       OMAP_HSMMC_WRITE(host->base, SYSCONFIG, SOFTRESET);
>> -       timeout = jiffies + msecs_to_jiffies(MMC_TIMEOUT_MS);
>> -       while ((OMAP_HSMMC_READ(host->base, SYSSTATUS) & RESETDONE) != RESETDONE
>> -               && time_before(jiffies, timeout))
>> -               ;
>> -
>> -       OMAP_HSMMC_WRITE(host->base, SYSCONFIG,
>> -                       OMAP_HSMMC_READ(host->base, SYSCONFIG) | AUTOIDLE);
>> +       if (!OMAP_HSMMC_READ(host->base, SYSSTATUS) & RESETDONE)
>> +               return 1;
> Should this check be removed.
>
No - it's still needed to check that the IP is ready for use.

>>
>>         if (host->pdata->controller_flags & OMAP_HSMMC_SUPPORTS_DUAL_VOLT) {
>>                 if (host->power_mode != MMC_POWER_OFF &&
>> @@ -1593,10 +1579,6 @@ static void omap_hsmmc_conf_bus_power(struct omap_hsmmc_host *host)
>>         value = OMAP_HSMMC_READ(host->base, CAPA);
>>         OMAP_HSMMC_WRITE(host->base, CAPA, value | capa);
>>
>> -       /* Set the controller to AUTO IDLE mode */
>> -       value = OMAP_HSMMC_READ(host->base, SYSCONFIG);
>> -       OMAP_HSMMC_WRITE(host->base, SYSCONFIG, value | AUTOIDLE);
>> -
>>         /* Set SD bus power bit */
>>         set_sd_bus_power(host);
>>  }
>> @@ -1654,8 +1636,6 @@ static int omap_hsmmc_regs_show(struct seq_file *s, void *data)
>>
>>         pm_runtime_get_sync(host->dev);
>>
>> -       seq_printf(s, "SYSCONFIG:\t0x%08x\n",
>> -                       OMAP_HSMMC_READ(host->base, SYSCONFIG));
>>         seq_printf(s, "CON:\t\t0x%08x\n",
>>                         OMAP_HSMMC_READ(host->base, CON));
>>         seq_printf(s, "HCTL:\t\t0x%08x\n",
>> --
>> 1.7.11.1.25.g0e18bef
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 07/10] mmc: omap_hsmmc: consolidate flush posted writes for HSMMC IRQs

[Venkatraman S]

Flushing spurious IRQs from HSMMC IP is done twice in
omap_hsmmc_irq and omap_hsmmc_do_irq.
Consolidate them to one location.

Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
 drivers/mmc/host/omap_hsmmc.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 4bc55ac..20453c8 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -969,15 +969,6 @@ static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
 	struct mmc_data *data;
 	int end_cmd = 0, end_trans = 0;
 
-	if (!host->req_in_progress) {
-		do {
-			OMAP_HSMMC_WRITE(host->base, STAT, status);
-			/* Flush posted write */
-			status = OMAP_HSMMC_READ(host->base, STAT);
-		} while (status & INT_EN_MASK);
-		return;
-	}
-
 	data = host->data;
 	dev_vdbg(mmc_dev(host->mmc), "IRQ Status is %x\n", status);
 
@@ -1028,8 +1019,6 @@ static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
 		}
 	}
 
-	OMAP_HSMMC_WRITE(host->base, STAT, status);
-
 	if (end_cmd || ((status & CC) && host->cmd))
 		omap_hsmmc_cmd_done(host, host->cmd);
 	if ((end_trans || (status & TC)) && host->mrq)
@@ -1045,11 +1034,13 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)
 	int status;
 
 	status = OMAP_HSMMC_READ(host->base, STAT);
-	do {
+	while (status & INT_EN_MASK && host->req_in_progress) {
 		omap_hsmmc_do_irq(host, status);
+
 		/* Flush posted write */
+		OMAP_HSMMC_WRITE(host->base, STAT, status);
 		status = OMAP_HSMMC_READ(host->base, STAT);
-	} while (status & INT_EN_MASK);
+	}
 
 	return IRQ_HANDLED;
 }
-- 
1.7.11.1.25.g0e18bef

Re: [PATCH 07/10] mmc: omap_hsmmc: consolidate flush posted writes for HSMMC IRQs

[T Krishnamoorthy, Balaji]

On Sat, Aug 18, 2012 at 12:22 AM, Venkatraman S <svenkatr@ti.com> wrote:
> Flushing spurious IRQs from HSMMC IP is done twice in
> omap_hsmmc_irq and omap_hsmmc_do_irq.

spurious IRQ is flushed in start of omap_hsmmc_do_irq
and irq acked at the end of omap_hsmmc_do_irq

> Consolidate them to one location.

if you wanted to consolidated ...

>
> Signed-off-by: Venkatraman S <svenkatr@ti.com>
> ---
>  drivers/mmc/host/omap_hsmmc.c | 17 ++++-------------
>  1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 4bc55ac..20453c8 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -969,15 +969,6 @@ static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
>         struct mmc_data *data;
>         int end_cmd = 0, end_trans = 0;
>
> -       if (!host->req_in_progress) {

just do a return from here

> -               do {
> -                       OMAP_HSMMC_WRITE(host->base, STAT, status);
> -                       /* Flush posted write */
> -                       status = OMAP_HSMMC_READ(host->base, STAT);
> -               } while (status & INT_EN_MASK);
> -               return;
> -       }
> -
>         data = host->data;
>         dev_vdbg(mmc_dev(host->mmc), "IRQ Status is %x\n", status);
>
> @@ -1028,8 +1019,6 @@ static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
>                 }
>         }
>
> -       OMAP_HSMMC_WRITE(host->base, STAT, status);
> -
>         if (end_cmd || ((status & CC) && host->cmd))
>                 omap_hsmmc_cmd_done(host, host->cmd);
>         if ((end_trans || (status & TC)) && host->mrq)
> @@ -1045,11 +1034,13 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)
>         int status;
>
>         status = OMAP_HSMMC_READ(host->base, STAT);
> -       do {
> +       while (status & INT_EN_MASK && host->req_in_progress) {

and remove the check for host->req_in_progress
I think do while loop can be retained as it will get executed once anyway.

>                 omap_hsmmc_do_irq(host, status);
> +

>                 /* Flush posted write */

comment is misplaced

> +               OMAP_HSMMC_WRITE(host->base, STAT, status);
>                 status = OMAP_HSMMC_READ(host->base, STAT);
> -       } while (status & INT_EN_MASK);
> +       }
>
>         return IRQ_HANDLED;
>  }
> --
> 1.7.11.1.25.g0e18bef
>

Re: [PATCH 07/10] mmc: omap_hsmmc: consolidate flush posted writes for HSMMC IRQs

[S, Venkatraman]

On Tue, Aug 21, 2012 at 8:51 PM, T Krishnamoorthy, Balaji
<balajitk@ti.com> wrote:
> On Sat, Aug 18, 2012 at 12:22 AM, Venkatraman S <svenkatr@ti.com> wrote:
>> Flushing spurious IRQs from HSMMC IP is done twice in
>> omap_hsmmc_irq and omap_hsmmc_do_irq.
>
> spurious IRQ is flushed in start of omap_hsmmc_do_irq
> and irq acked at the end of omap_hsmmc_do_irq
>
>> Consolidate them to one location.
>
> if you wanted to consolidated ...
>
>>
>> Signed-off-by: Venkatraman S <svenkatr@ti.com>
>> ---
>>  drivers/mmc/host/omap_hsmmc.c | 17 ++++-------------
>>  1 file changed, 4 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
>> index 4bc55ac..20453c8 100644
>> --- a/drivers/mmc/host/omap_hsmmc.c
>> +++ b/drivers/mmc/host/omap_hsmmc.c
>> @@ -969,15 +969,6 @@ static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
>>         struct mmc_data *data;
>>         int end_cmd = 0, end_trans = 0;
>>
>> -       if (!host->req_in_progress) {
>
> just do a return from here

I think it's still better to filter out the validation to the outer
function and let do_irq to just 'do' it.

>
>> -               do {
>> -                       OMAP_HSMMC_WRITE(host->base, STAT, status);
>> -                       /* Flush posted write */
>> -                       status = OMAP_HSMMC_READ(host->base, STAT);
>> -               } while (status & INT_EN_MASK);
>> -               return;
>> -       }
>> -
>>         data = host->data;
>>         dev_vdbg(mmc_dev(host->mmc), "IRQ Status is %x\n", status);
>>
>> @@ -1028,8 +1019,6 @@ static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
>>                 }
>>         }
>>
>> -       OMAP_HSMMC_WRITE(host->base, STAT, status);
>> -
>>         if (end_cmd || ((status & CC) && host->cmd))
>>                 omap_hsmmc_cmd_done(host, host->cmd);
>>         if ((end_trans || (status & TC)) && host->mrq)
>> @@ -1045,11 +1034,13 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)
>>         int status;
>>
>>         status = OMAP_HSMMC_READ(host->base, STAT);
>> -       do {
>> +       while (status & INT_EN_MASK && host->req_in_progress) {
>
> and remove the check for host->req_in_progress
> I think do while loop can be retained as it will get executed once anyway.

It's a minor issue, and the consolidation of the check for
req_in_progress makes sure that it might
sometimes be not executed at all..


>
>>                 omap_hsmmc_do_irq(host, status);
>> +
>
>>                 /* Flush posted write */
>
> comment is misplaced
>
Ok. I'll remove.

>> +               OMAP_HSMMC_WRITE(host->base, STAT, status);
>>                 status = OMAP_HSMMC_READ(host->base, STAT);
>> -       } while (status & INT_EN_MASK);
>> +       }
>>
>>         return IRQ_HANDLED;
>>  }
>> --
>> 1.7.11.1.25.g0e18bef
>>

[PATCH 08/10] mmc: omap_hsmmc: consolidate error report handling of HSMMC IRQ

[Venkatraman S]

Consolidate the duplicated code around the handling of CMD_TIMEOUT,
CMD_CRC, DATA_TIMEOUT, DATA_CRC and CARD_ERR handling into a
single function.
This generally shrinks code bloat, but is also required for implementing
software based guard timers.

Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
 drivers/mmc/host/omap_hsmmc.c | 63 +++++++++++++++----------------------------
 1 file changed, 21 insertions(+), 42 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 20453c8..9afdd20 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -964,6 +964,18 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host,
 			__func__);
 }
 
+static void hsmmc_command_incomplete(struct omap_hsmmc_host *host, int err)
+{
+	omap_hsmmc_reset_controller_fsm(host, SRC);
+	host->cmd->error = err;
+
+	if (host->data) {
+		omap_hsmmc_reset_controller_fsm(host, SRD);
+		omap_hsmmc_dma_cleanup(host, err);
+	}
+
+}
+
 static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
 {
 	struct mmc_data *data;
@@ -974,48 +986,15 @@ static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
 
 	if (status & ERR) {
 		omap_hsmmc_dbg_report_irq(host, status);
-		if ((status & CMD_TIMEOUT) ||
-			(status & CMD_CRC)) {
-			if (host->cmd) {
-				if (status & CMD_TIMEOUT) {
-					omap_hsmmc_reset_controller_fsm(host,
-									SRC);
-					host->cmd->error = -ETIMEDOUT;
-				} else {
-					host->cmd->error = -EILSEQ;
-				}
-				end_cmd = 1;
-			}
-			if (host->data || host->response_busy) {
-				if (host->data)
-					omap_hsmmc_dma_cleanup(host,
-								-ETIMEDOUT);
-				host->response_busy = 0;
-				omap_hsmmc_reset_controller_fsm(host, SRD);
-			}
-		}
-		if ((status & DATA_TIMEOUT) ||
-			(status & DATA_CRC)) {
-			if (host->data || host->response_busy) {
-				int err = (status & DATA_TIMEOUT) ?
-						-ETIMEDOUT : -EILSEQ;
-
-				if (host->data)
-					omap_hsmmc_dma_cleanup(host, err);
-				else
-					host->mrq->cmd->error = err;
-				host->response_busy = 0;
-				omap_hsmmc_reset_controller_fsm(host, SRD);
-				end_trans = 1;
-			}
-		}
-		if (status & CARD_ERR) {
-			dev_dbg(mmc_dev(host->mmc),
-				"Ignoring card err CMD%d\n", host->cmd->opcode);
-			if (host->cmd)
-				end_cmd = 1;
-			if (host->data)
-				end_trans = 1;
+		if (status & (CMD_TIMEOUT | DATA_TIMEOUT))
+			hsmmc_command_incomplete(host, -ETIMEDOUT);
+		else if (status & (CMD_CRC | DATA_CRC))
+			hsmmc_command_incomplete(host, -EILSEQ);
+
+		end_cmd = 1;
+		if (host->data || host->response_busy) {
+			end_trans = 1;
+			host->response_busy = 0;
 		}
 	}
 
-- 
1.7.11.1.25.g0e18bef

[PATCH 09/10] mmc: omap_hsmmc: convert from IP timer to hrtimer

[Venkatraman S]

omap hsmmc controller IP has an inbuilt timer that can be programmed to
guard against unresponsive operations. But it's range is very narrow,
and it's maximum countable time is a few seconds.

Card maintenance operations like BKOPS and SECURE DISCARD and long
stream writes like packed command require timers of order of
several minutes.
So get rid of using the IP timer entirely and use kernel's hrtimer
functionality for guarding the device operations.
As part of this change, a workaround that disabled timeouts for
MMC_ERASE commands is removed, and the arbitary timing of 100ms
is used only when the timeout is not explicitly specified by core.

Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
 drivers/mmc/host/omap_hsmmc.c | 96 ++++++++++++++++++++++---------------------
 1 file changed, 50 insertions(+), 46 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 9afdd20..8f7cebc 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -79,7 +79,7 @@
 #define CLKD_SHIFT		6
 #define DTO_MASK		0x000F0000
 #define DTO_SHIFT		16
-#define INT_EN_MASK		0x307F0033
+#define INT_EN_MASK		0x306E0033
 #define BWR_ENABLE		(1 << 4)
 #define BRR_ENABLE		(1 << 5)
 #define DTO_ENABLE		(1 << 20)
@@ -160,6 +160,7 @@ struct omap_hsmmc_host {
 	unsigned int		dma_sg_idx;
 	unsigned char		bus_mode;
 	unsigned char		power_mode;
+	unsigned int		ns_per_clk_cycle;
 	int			suspended;
 	int			irq;
 	int			use_dma, dma_ch;
@@ -172,6 +173,7 @@ struct omap_hsmmc_host {
 	int			reqs_blocked;
 	int			use_reg;
 	int			req_in_progress;
+	struct hrtimer	guard_timer;
 	struct omap_hsmmc_next	next_data;
 
 	struct	omap_mmc_platform_data	*pdata;
@@ -455,10 +457,6 @@ static void omap_hsmmc_enable_irq(struct omap_hsmmc_host *host,
 	else
 		irq_mask = INT_EN_MASK;
 
-	/* Disable timeout for erases */
-	if (cmd->opcode == MMC_ERASE)
-		irq_mask &= ~DTO_ENABLE;
-
 	OMAP_HSMMC_WRITE(host->base, STAT, STAT_CLEAR);
 	OMAP_HSMMC_WRITE(host->base, ISE, irq_mask);
 	OMAP_HSMMC_WRITE(host->base, IE, irq_mask);
@@ -508,6 +506,9 @@ static void omap_hsmmc_set_clock(struct omap_hsmmc_host *host)
 		&& time_before(jiffies, timeout))
 		cpu_relax();
 
+	if (ios->clock)
+		host->ns_per_clk_cycle = DIV_ROUND_UP(NSEC_PER_SEC, ios->clock);
+
 	omap_hsmmc_start_clock(host);
 }
 
@@ -824,7 +825,7 @@ omap_hsmmc_xfer_done(struct omap_hsmmc_host *host, struct mmc_data *data)
 		omap_hsmmc_request_done(host, mrq);
 		return;
 	}
-
+	hrtimer_cancel(&host->guard_timer);
 	host->data = NULL;
 
 	if (!data->error)
@@ -859,8 +860,11 @@ omap_hsmmc_cmd_done(struct omap_hsmmc_host *host, struct mmc_command *cmd)
 			cmd->resp[0] = OMAP_HSMMC_READ(host->base, RSP10);
 		}
 	}
-	if ((host->data == NULL && !host->response_busy) || cmd->error)
+	if ((host->data == NULL && !host->response_busy) || cmd->error) {
+		if (cmd->error != -ETIMEDOUT)
+			hrtimer_cancel(&host->guard_timer);
 		omap_hsmmc_request_done(host, cmd->mrq);
+	}
 }
 
 /*
@@ -992,7 +996,7 @@ static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
 			hsmmc_command_incomplete(host, -EILSEQ);
 
 		end_cmd = 1;
-		if (host->data || host->response_busy) {
+		if (data || host->response_busy) {
 			end_trans = 1;
 			host->response_busy = 0;
 		}
@@ -1292,41 +1296,35 @@ static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host,
 	return 0;
 }
 
-static void set_data_timeout(struct omap_hsmmc_host *host,
-			     unsigned int timeout_ns,
-			     unsigned int timeout_clks)
+static void set_guard_timer(struct omap_hsmmc_host *host,
+		unsigned long timeout_ms, unsigned long timeout_ns,
+		unsigned int timeout_clks)
 {
-	unsigned int timeout, cycle_ns;
-	uint32_t reg, clkd, dto = 0;
+	ktime_t gtime;
+	unsigned int sec, nsec;
 
-	reg = OMAP_HSMMC_READ(host->base, SYSCTL);
-	clkd = (reg & CLKD_MASK) >> CLKD_SHIFT;
-	if (clkd == 0)
-		clkd = 1;
+	sec = timeout_ms / MSEC_PER_SEC;
+	nsec = (timeout_ms % MSEC_PER_SEC) * NSEC_PER_MSEC + timeout_ns;
 
-	cycle_ns = 1000000000 / (clk_get_rate(host->fclk) / clkd);
-	timeout = timeout_ns / cycle_ns;
-	timeout += timeout_clks;
-	if (timeout) {
-		while ((timeout & 0x80000000) == 0) {
-			dto += 1;
-			timeout <<= 1;
-		}
-		dto = 31 - dto;
-		timeout <<= 1;
-		if (timeout && dto)
-			dto += 1;
-		if (dto >= 13)
-			dto -= 13;
-		else
-			dto = 0;
-		if (dto > 14)
-			dto = 14;
-	}
+	nsec += timeout_clks * host->ns_per_clk_cycle;
+	gtime = ktime_set(sec, nsec);
 
-	reg &= ~DTO_MASK;
-	reg |= dto << DTO_SHIFT;
-	OMAP_HSMMC_WRITE(host->base, SYSCTL, reg);
+	hrtimer_start(&host->guard_timer, gtime, HRTIMER_MODE_REL);
+}
+
+enum hrtimer_restart omap_hsmmc_timedout(struct hrtimer *gtimer)
+{
+	struct omap_hsmmc_host *host;
+	host = container_of(gtimer, struct omap_hsmmc_host, guard_timer);
+
+	omap_hsmmc_disable_irq(host);
+	hsmmc_command_incomplete(host, -ETIMEDOUT);
+	host->response_busy = 0;
+	omap_hsmmc_cmd_done(host, host->cmd);
+	if (host->data)
+		omap_hsmmc_xfer_done(host, host->data);
+
+	return HRTIMER_NORESTART;
 }
 
 /*
@@ -1340,18 +1338,22 @@ omap_hsmmc_prepare_data(struct omap_hsmmc_host *host, struct mmc_request *req)
 
 	if (req->data == NULL) {
 		OMAP_HSMMC_WRITE(host->base, BLK, 0);
-		/*
-		 * Set an arbitrary 100ms data timeout for commands with
-		 * busy signal.
-		 */
-		if (req->cmd->flags & MMC_RSP_BUSY)
-			set_data_timeout(host, 100000000U, 0);
+		if (req->cmd->cmd_timeout_ms == 0) {
+			/*
+			 * Set an arbitrary 100ms data timeout for commands with
+			 * busy signal.
+			 */
+			set_guard_timer(host, 100, 0, 0);
+		} else {
+			set_guard_timer(host, req->cmd->cmd_timeout_ms, 0, 0);
+		}
 		return 0;
 	}
 
 	OMAP_HSMMC_WRITE(host->base, BLK, (req->data->blksz)
 					| (req->data->blocks << 16));
-	set_data_timeout(host, req->data->timeout_ns, req->data->timeout_clks);
+	set_guard_timer(host, 0, req->data->timeout_ns,
+			req->data->timeout_clks);
 
 	if (host->use_dma) {
 		ret = omap_hsmmc_start_dma_transfer(host, req);
@@ -1921,6 +1923,8 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
 		pdata->suspend = omap_hsmmc_suspend_cdirq;
 		pdata->resume = omap_hsmmc_resume_cdirq;
 	}
+	hrtimer_init(&host->guard_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	host->guard_timer.function = omap_hsmmc_timedout;
 
 	omap_hsmmc_disable_irq(host);
 
-- 
1.7.11.1.25.g0e18bef

Re: [PATCH 09/10] mmc: omap_hsmmc: convert from IP timer to hrtimer

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 5702 bytes --]

On Sat, Aug 18, 2012 at 12:22:29AM +0530, Venkatraman S wrote:
> omap hsmmc controller IP has an inbuilt timer that can be programmed to
                                  ^^^^^^^
				  built-in
> guard against unresponsive operations. But it's range is very narrow,
                                             ^^^^
					     its


> and it's maximum countable time is a few seconds.
      ^^^^
      its


> Card maintenance operations like BKOPS and SECURE DISCARD and long
> stream writes like packed command require timers of order of
> several minutes.
> So get rid of using the IP timer entirely and use kernel's hrtimer
> functionality for guarding the device operations.
> As part of this change, a workaround that disabled timeouts for
> MMC_ERASE commands is removed, and the arbitary timing of 100ms
> is used only when the timeout is not explicitly specified by core.
> 
> Signed-off-by: Venkatraman S <svenkatr@ti.com>
> ---
>  drivers/mmc/host/omap_hsmmc.c | 96 ++++++++++++++++++++++---------------------
>  1 file changed, 50 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 9afdd20..8f7cebc 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -79,7 +79,7 @@
>  #define CLKD_SHIFT		6
>  #define DTO_MASK		0x000F0000
>  #define DTO_SHIFT		16
> -#define INT_EN_MASK		0x307F0033
> +#define INT_EN_MASK		0x306E0033

not related to this patch in particular, but it would be nice if this
was converted to something more meaningfull, like ORing a bunch of bit
defines.

>  #define BWR_ENABLE		(1 << 4)
>  #define BRR_ENABLE		(1 << 5)
>  #define DTO_ENABLE		(1 << 20)
> @@ -160,6 +160,7 @@ struct omap_hsmmc_host {
>  	unsigned int		dma_sg_idx;
>  	unsigned char		bus_mode;
>  	unsigned char		power_mode;
> +	unsigned int		ns_per_clk_cycle;
>  	int			suspended;
>  	int			irq;
>  	int			use_dma, dma_ch;
> @@ -172,6 +173,7 @@ struct omap_hsmmc_host {
>  	int			reqs_blocked;
>  	int			use_reg;
>  	int			req_in_progress;
> +	struct hrtimer	guard_timer;
>  	struct omap_hsmmc_next	next_data;
>  
>  	struct	omap_mmc_platform_data	*pdata;
> @@ -455,10 +457,6 @@ static void omap_hsmmc_enable_irq(struct omap_hsmmc_host *host,
>  	else
>  		irq_mask = INT_EN_MASK;
>  
> -	/* Disable timeout for erases */
> -	if (cmd->opcode == MMC_ERASE)
> -		irq_mask &= ~DTO_ENABLE;
> -
>  	OMAP_HSMMC_WRITE(host->base, STAT, STAT_CLEAR);
>  	OMAP_HSMMC_WRITE(host->base, ISE, irq_mask);
>  	OMAP_HSMMC_WRITE(host->base, IE, irq_mask);
> @@ -508,6 +506,9 @@ static void omap_hsmmc_set_clock(struct omap_hsmmc_host *host)
>  		&& time_before(jiffies, timeout))
>  		cpu_relax();
>  
> +	if (ios->clock)
> +		host->ns_per_clk_cycle = DIV_ROUND_UP(NSEC_PER_SEC, ios->clock);
> +
>  	omap_hsmmc_start_clock(host);
>  }
>  
> @@ -824,7 +825,7 @@ omap_hsmmc_xfer_done(struct omap_hsmmc_host *host, struct mmc_data *data)
>  		omap_hsmmc_request_done(host, mrq);
>  		return;
>  	}
> -
> +	hrtimer_cancel(&host->guard_timer);
>  	host->data = NULL;
>  
>  	if (!data->error)
> @@ -859,8 +860,11 @@ omap_hsmmc_cmd_done(struct omap_hsmmc_host *host, struct mmc_command *cmd)
>  			cmd->resp[0] = OMAP_HSMMC_READ(host->base, RSP10);
>  		}
>  	}
> -	if ((host->data == NULL && !host->response_busy) || cmd->error)
> +	if ((host->data == NULL && !host->response_busy) || cmd->error) {

could just go ahead and make this check uniform by:

if ((!host->data && !host->response_busy)) || cmd->error)

> +		if (cmd->error != -ETIMEDOUT)
> +			hrtimer_cancel(&host->guard_timer);
>  		omap_hsmmc_request_done(host, cmd->mrq);
> +	}
>  }
>  
>  /*
> @@ -992,7 +996,7 @@ static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
>  			hsmmc_command_incomplete(host, -EILSEQ);
>  
>  		end_cmd = 1;
> -		if (host->data || host->response_busy) {
> +		if (data || host->response_busy) {

This doesn't seem like it belongs to $SUBJECT...

>  			end_trans = 1;
>  			host->response_busy = 0;
>  		}
> @@ -1292,41 +1296,35 @@ static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host,
>  	return 0;
>  }
>  
> -static void set_data_timeout(struct omap_hsmmc_host *host,
> -			     unsigned int timeout_ns,
> -			     unsigned int timeout_clks)
> +static void set_guard_timer(struct omap_hsmmc_host *host,
> +		unsigned long timeout_ms, unsigned long timeout_ns,
> +		unsigned int timeout_clks)
>  {
> -	unsigned int timeout, cycle_ns;
> -	uint32_t reg, clkd, dto = 0;
> +	ktime_t gtime;
> +	unsigned int sec, nsec;
>  
> -	reg = OMAP_HSMMC_READ(host->base, SYSCTL);
> -	clkd = (reg & CLKD_MASK) >> CLKD_SHIFT;
> -	if (clkd == 0)
> -		clkd = 1;
> +	sec = timeout_ms / MSEC_PER_SEC;
> +	nsec = (timeout_ms % MSEC_PER_SEC) * NSEC_PER_MSEC + timeout_ns;
>  
> -	cycle_ns = 1000000000 / (clk_get_rate(host->fclk) / clkd);
> -	timeout = timeout_ns / cycle_ns;
> -	timeout += timeout_clks;
> -	if (timeout) {
> -		while ((timeout & 0x80000000) == 0) {
> -			dto += 1;
> -			timeout <<= 1;
> -		}
> -		dto = 31 - dto;
> -		timeout <<= 1;
> -		if (timeout && dto)
> -			dto += 1;
> -		if (dto >= 13)
> -			dto -= 13;
> -		else
> -			dto = 0;
> -		if (dto > 14)
> -			dto = 14;
> -	}
> +	nsec += timeout_clks * host->ns_per_clk_cycle;
> +	gtime = ktime_set(sec, nsec);
>  
> -	reg &= ~DTO_MASK;
> -	reg |= dto << DTO_SHIFT;
> -	OMAP_HSMMC_WRITE(host->base, SYSCTL, reg);
> +	hrtimer_start(&host->guard_timer, gtime, HRTIMER_MODE_REL);
> +}
> +
> +enum hrtimer_restart omap_hsmmc_timedout(struct hrtimer *gtimer)

static ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 09/10] mmc: omap_hsmmc: convert from IP timer to hrtimer

[S, Venkatraman]

On Tue, Aug 21, 2012 at 4:12 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Sat, Aug 18, 2012 at 12:22:29AM +0530, Venkatraman S wrote:
>> omap hsmmc controller IP has an inbuilt timer that can be programmed to
>                                   ^^^^^^^
>                                   built-in
>> guard against unresponsive operations. But it's range is very narrow,
>                                              ^^^^
>                                              its
>
>
>> and it's maximum countable time is a few seconds.
>       ^^^^
>       its
>

Will fix.

>
>> Card maintenance operations like BKOPS and SECURE DISCARD and long
>> stream writes like packed command require timers of order of
>> several minutes.
>> So get rid of using the IP timer entirely and use kernel's hrtimer
>> functionality for guarding the device operations.
>> As part of this change, a workaround that disabled timeouts for
>> MMC_ERASE commands is removed, and the arbitary timing of 100ms
>> is used only when the timeout is not explicitly specified by core.
>>
>> Signed-off-by: Venkatraman S <svenkatr@ti.com>
>> ---
>>  drivers/mmc/host/omap_hsmmc.c | 96 ++++++++++++++++++++++---------------------
>>  1 file changed, 50 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
>> index 9afdd20..8f7cebc 100644
>> --- a/drivers/mmc/host/omap_hsmmc.c
>> +++ b/drivers/mmc/host/omap_hsmmc.c
>> @@ -79,7 +79,7 @@
>>  #define CLKD_SHIFT           6
>>  #define DTO_MASK             0x000F0000
>>  #define DTO_SHIFT            16
>> -#define INT_EN_MASK          0x307F0033
>> +#define INT_EN_MASK          0x306E0033
>
> not related to this patch in particular, but it would be nice if this
> was converted to something more meaningfull, like ORing a bunch of bit
> defines.
>

Sure. Good to do now as part of the change.

>>  #define BWR_ENABLE           (1 << 4)
>>  #define BRR_ENABLE           (1 << 5)
>>  #define DTO_ENABLE           (1 << 20)
>> @@ -160,6 +160,7 @@ struct omap_hsmmc_host {
>>       unsigned int            dma_sg_idx;
>>       unsigned char           bus_mode;
>>       unsigned char           power_mode;
>> +     unsigned int            ns_per_clk_cycle;
>>       int                     suspended;
>>       int                     irq;
>>       int                     use_dma, dma_ch;
>> @@ -172,6 +173,7 @@ struct omap_hsmmc_host {
>>       int                     reqs_blocked;
>>       int                     use_reg;
>>       int                     req_in_progress;
>> +     struct hrtimer  guard_timer;
>>       struct omap_hsmmc_next  next_data;
>>
>>       struct  omap_mmc_platform_data  *pdata;
>> @@ -455,10 +457,6 @@ static void omap_hsmmc_enable_irq(struct omap_hsmmc_host *host,
>>       else
>>               irq_mask = INT_EN_MASK;
>>
>> -     /* Disable timeout for erases */
>> -     if (cmd->opcode == MMC_ERASE)
>> -             irq_mask &= ~DTO_ENABLE;
>> -
>>       OMAP_HSMMC_WRITE(host->base, STAT, STAT_CLEAR);
>>       OMAP_HSMMC_WRITE(host->base, ISE, irq_mask);
>>       OMAP_HSMMC_WRITE(host->base, IE, irq_mask);
>> @@ -508,6 +506,9 @@ static void omap_hsmmc_set_clock(struct omap_hsmmc_host *host)
>>               && time_before(jiffies, timeout))
>>               cpu_relax();
>>
>> +     if (ios->clock)
>> +             host->ns_per_clk_cycle = DIV_ROUND_UP(NSEC_PER_SEC, ios->clock);
>> +
>>       omap_hsmmc_start_clock(host);
>>  }
>>
>> @@ -824,7 +825,7 @@ omap_hsmmc_xfer_done(struct omap_hsmmc_host *host, struct mmc_data *data)
>>               omap_hsmmc_request_done(host, mrq);
>>               return;
>>       }
>> -
>> +     hrtimer_cancel(&host->guard_timer);
>>       host->data = NULL;
>>
>>       if (!data->error)
>> @@ -859,8 +860,11 @@ omap_hsmmc_cmd_done(struct omap_hsmmc_host *host, struct mmc_command *cmd)
>>                       cmd->resp[0] = OMAP_HSMMC_READ(host->base, RSP10);
>>               }
>>       }
>> -     if ((host->data == NULL && !host->response_busy) || cmd->error)
>> +     if ((host->data == NULL && !host->response_busy) || cmd->error) {
>
> could just go ahead and make this check uniform by:
>
> if ((!host->data && !host->response_busy)) || cmd->error)
>

Ok.

>> +             if (cmd->error != -ETIMEDOUT)
>> +                     hrtimer_cancel(&host->guard_timer);
>>               omap_hsmmc_request_done(host, cmd->mrq);
>> +     }
>>  }
>>
>>  /*
>> @@ -992,7 +996,7 @@ static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
>>                       hsmmc_command_incomplete(host, -EILSEQ);
>>
>>               end_cmd = 1;
>> -             if (host->data || host->response_busy) {
>> +             if (data || host->response_busy) {
>
> This doesn't seem like it belongs to $SUBJECT...
>
I thought is was a small fix which didn't warrant a separate patch though.

>>                       end_trans = 1;
>>                       host->response_busy = 0;
>>               }
>> @@ -1292,41 +1296,35 @@ static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host,
>>       return 0;
>>  }
>>
>> -static void set_data_timeout(struct omap_hsmmc_host *host,
>> -                          unsigned int timeout_ns,
>> -                          unsigned int timeout_clks)
>> +static void set_guard_timer(struct omap_hsmmc_host *host,
>> +             unsigned long timeout_ms, unsigned long timeout_ns,
>> +             unsigned int timeout_clks)
>>  {
>> -     unsigned int timeout, cycle_ns;
>> -     uint32_t reg, clkd, dto = 0;
>> +     ktime_t gtime;
>> +     unsigned int sec, nsec;
>>
>> -     reg = OMAP_HSMMC_READ(host->base, SYSCTL);
>> -     clkd = (reg & CLKD_MASK) >> CLKD_SHIFT;
>> -     if (clkd == 0)
>> -             clkd = 1;
>> +     sec = timeout_ms / MSEC_PER_SEC;
>> +     nsec = (timeout_ms % MSEC_PER_SEC) * NSEC_PER_MSEC + timeout_ns;
>>
>> -     cycle_ns = 1000000000 / (clk_get_rate(host->fclk) / clkd);
>> -     timeout = timeout_ns / cycle_ns;
>> -     timeout += timeout_clks;
>> -     if (timeout) {
>> -             while ((timeout & 0x80000000) == 0) {
>> -                     dto += 1;
>> -                     timeout <<= 1;
>> -             }
>> -             dto = 31 - dto;
>> -             timeout <<= 1;
>> -             if (timeout && dto)
>> -                     dto += 1;
>> -             if (dto >= 13)
>> -                     dto -= 13;
>> -             else
>> -                     dto = 0;
>> -             if (dto > 14)
>> -                     dto = 14;
>> -     }
>> +     nsec += timeout_clks * host->ns_per_clk_cycle;
>> +     gtime = ktime_set(sec, nsec);
>>
>> -     reg &= ~DTO_MASK;
>> -     reg |= dto << DTO_SHIFT;
>> -     OMAP_HSMMC_WRITE(host->base, SYSCTL, reg);
>> +     hrtimer_start(&host->guard_timer, gtime, HRTIMER_MODE_REL);
>> +}
>> +
>> +enum hrtimer_restart omap_hsmmc_timedout(struct hrtimer *gtimer)
>
> static ?
>

Right!

Re: [PATCH 09/10] mmc: omap_hsmmc: convert from IP timer to hrtimer

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 759 bytes --]

Hi,

On Wed, Aug 22, 2012 at 04:08:17PM +0530, S, Venkatraman wrote:
> >> @@ -992,7 +996,7 @@ static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
> >>                       hsmmc_command_incomplete(host, -EILSEQ);
> >>
> >>               end_cmd = 1;
> >> -             if (host->data || host->response_busy) {
> >> +             if (data || host->response_busy) {
> >
> > This doesn't seem like it belongs to $SUBJECT...
> >
> I thought is was a small fix which didn't warrant a separate patch though.

sure, that's fair. But it does deserve a mention on commit log too.
Something along the lines of "while at that, also use a 'data' variable
which was already defined instead of dereferencing host again".

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 10/10] mmc: omap_hsmmc: Move to Maintained state in MAINTAINERS

[Venkatraman S]

I can continue to look after this driver.

Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
 MAINTAINERS | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 72c2681..75e3c3e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4933,8 +4933,10 @@ S:	Maintained
 F:	drivers/mmc/host/omap.c
 
 OMAP HS MMC SUPPORT
+M:	Venkatraman S <svenkatr@ti.com>
+L:	linux-mmc@vger.kernel.org
 L:	linux-omap@vger.kernel.org
-S:	Orphan
+S:	Maintained
 F:	drivers/mmc/host/omap_hsmmc.c
 
 OMAP RANDOM NUMBER GENERATOR SUPPORT
-- 
1.7.11.1.25.g0e18bef

Re: [PATCH 00/10] Assorted MMC / OMAP HSMMC patches

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1856 bytes --]

Hi,

On Sat, Aug 18, 2012 at 12:22:20AM +0530, Venkatraman S wrote:
> Essentially, a lot of cleanups leading up to adding a new
> feature for OMAP HSMMC. The idea is to convert to the use
> of software timer instead of IP timer for timekeeping, due
> to the limitations of the counting range of the IP timer.
> 
> Also added myself as OMAP HSMMC maintainer.
> 
> Patch 9/10 is in draft state and needs more testing.
> 
> These patches are also available at
>   git://github.com/svenkatr/linux.git my/mmc/3.6/hrtimer_updates

other than the small comments I had to patch 9, this series looks very
good:

Acked-by: Felipe Balbi <balbi@ti.com>

> 
> Venkatraman S (10):
>   mmc: core: Add TRANsfer state to non-HPI state
>   mmc: debugfs: Print ext_csd in ascending order
>   mmc: omap: remove unused variables and includes
>   mmc: omap: fix mmc_omap_report_irq to use dev_dbg macros
>   mmc: omap_hsmmc: remove unused vars and includes
>   mmc: omap_hsmmc: remove access to SYSCONFIG register
>   mmc: omap_hsmmc: consolidate flush posted writes for HSMMC IRQs
>   mmc: omap_hsmmc: consolidate error report handling of HSMMC IRQ
>   mmc: omap_hsmmc: convert from IP timer to hrtimer
>   mmc: omap_hsmmc: Move to Maintained state in MAINTAINERS
> 
>  MAINTAINERS                   |   4 +-
>  drivers/mmc/core/core.c       |   3 +-
>  drivers/mmc/core/debugfs.c    |   2 +-
>  drivers/mmc/host/omap.c       |  39 ++++----
>  drivers/mmc/host/omap_hsmmc.c | 212 ++++++++++++++++--------------------------
>  5 files changed, 103 insertions(+), 157 deletions(-)
> 
> -- 
> 1.7.11.1.25.g0e18bef
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 00/10] Assorted MMC / OMAP HSMMC patches

[Chris Ball]

Hi,

On Tue, Aug 21 2012, Felipe Balbi wrote:
> On Sat, Aug 18, 2012 at 12:22:20AM +0530, Venkatraman S wrote:
>> Essentially, a lot of cleanups leading up to adding a new
>> feature for OMAP HSMMC. The idea is to convert to the use
>> of software timer instead of IP timer for timekeeping, due
>> to the limitations of the counting range of the IP timer.
>> 
>> Also added myself as OMAP HSMMC maintainer.
>> 
>> Patch 9/10 is in draft state and needs more testing.
>> 
>> These patches are also available at
>>   git://github.com/svenkatr/linux.git my/mmc/3.6/hrtimer_updates
>
> other than the small comments I had to patch 9, this series looks very
> good:
>
> Acked-by: Felipe Balbi <balbi@ti.com>

Thanks -- I've pushed everything except 9/10 to mmc-next for 3.7.
Venkat, please resubmit patch 9 individually whenever you're ready.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child