Re: [PATCH 09/10] mmc: omap_hsmmc: convert from IP timer to hrtimer
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
signature.asc
Description: Digital signature
Re: [PATCH 09/10] mmc: omap_hsmmc: convert from IP timer to hrtimer
On Tue, Aug 21, 2012 at 4:12 PM, Felipe Balbi 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
>> ---
>> 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 0x000F
>> #define DTO_SHIFT16
>> -#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 intdma_sg_idx;
>> unsigned char bus_mode;
>> unsigned char power_mode;
>> + unsigned intns_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 oma
Re: [PATCH 09/10] mmc: omap_hsmmc: convert from IP timer to hrtimer
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
> ---
> 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 0x000F
> #define DTO_SHIFT16
> -#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 intdma_sg_idx;
> unsigned char bus_mode;
> unsigned char power_mode;
> + unsigned intns_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,
> +
[PATCH 09/10] mmc: omap_hsmmc: convert from IP timer to hrtimer
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
---
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 0x000F
#define DTO_SHIFT 16
-#define INT_EN_MASK0x307F0033
+#define INT_EN_MASK0x306E0033
#define BWR_ENABLE (1 << 4)
#define BRR_ENABLE (1 << 5)
#define DTO_ENABLE (1 << 20)
@@ -160,6 +160,7 @@ struct omap_hsmmc_host {
unsigned intdma_sg_idx;
unsigned char bus_mode;
unsigned char power_mode;
+ unsigned intns_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 = 10 / (clk_get_rate(host->fclk) / clkd);
- timeout = timeout_ns / cycle_ns;
- timeout += timeout_clks;
- if (timeout) {
- while ((
