From 68c4c2d9db30fea39ab29ba29f22eab3fbcd47af Mon Sep 17 00:00:00 2001 From: Olivier Stoltz Douchet Date: Wed, 7 Dec 2011 18:05:39 +0100 Subject: [PATCH] [PORT FROM R2] hsi_ffl_tty.c: streamlining the TX timeout and stop TX timer management BZ: 15222 This patch is cleaning and factorising the TX timeout and stop TX timer management to limit the number of code sections where those timers are updated In more details, this patch is: - also directly killing the stop TX timer in the _ffl_from_wait_to_ctrl() function upon successful insertion of HSI message in the controller at the very same location where the TX timeout timer is updated; - adding a _ffl_update_state_tx() function aimed at factorising those timers management, mainly killing the hangup timer when there is no left HSI message in the controller and starting the stop TX timer when the full write pipeline is empty. This helper function is the sibling of the _ffl_update_state_rx() function; - this _ffl_update_state_tx() is now called wherever the wait FIFO and the controller FIFO are updated: in the _ffl_pop_wait_push_ctrl() function, which is transferring as much TX messages as possible from the wait FIFO and the controller as well as in the ffl_destruct_frame() callback; - the _ffl_tx_ctx_is_empty() helper function has been removed as it was only used once in this new code version, and its internal code has been directly carbon copied in the ffl_tx_full_pipe_is_clean() function; - the TX timeout and stop TX timer management has been removed from the ffl_complete_tx() function for it is now included in the _ffl_pop_wait_push_ctrl() call; - the ffl_throw_tty_hangup() function has been dropped as it was mistakenly used with a spinlock_irq_save in the ISR (which is bad) and was only used once elsewhere. The content of this initial function has been copied to the ISR without the _irqsave modifier, and was updated in the ffl_tty_tx_timeout() timer handler callback to only throw a TTY hangup if the HSI controller FIFO is not empty - which is safer than the current implementation. This latter modification is further ensuring that no unexpected TX timeout hangup can happen when the HSI controller FIFO is empty - maybe due to a race condition between the del_timer() and mod_timer() functions. This new hangup management is simpler than the previous one as it does not update the TX timeout timer upon reception on a frame, but only when adding a new frame in the controller. Finally, this new version is also adding better traces for RESET_OUT and CORE_DUMP, adding the cause information. In a nutshell, the purpose of this patch is to bind the TX timeout and stop TX timer management to ensure a full coherency in this respect, to simplify the TX timeout management and to filter out potential spurious TX timeout hangups. Change-Id: I0cc4e5ca720a1fee4060681780c59aeada476193 Orig-change-Id: Id53abc766cd9687321f3f21195fe7f898e24948f Orig-Change-Id: I6e1ce86acbde63f96b744d972a31fcb53afb1205 Signed-off-by: Olivier Stoltz Douchet Reviewed-on: http://android.intel.com:8080/31445 Reviewed-by: Predon, Frederic Reviewed-by: Lebsir, SamiX Tested-by: Lebsir, SamiX Reviewed-by: buildbot Tested-by: buildbot --- drivers/hsi/clients/hsi_ffl_tty.c | 126 +++++++++++++++++++------------------- 1 file changed, 64 insertions(+), 62 deletions(-) diff --git a/drivers/hsi/clients/hsi_ffl_tty.c b/drivers/hsi/clients/hsi_ffl_tty.c index 9586502..75545c1 100644 --- a/drivers/hsi/clients/hsi_ffl_tty.c +++ b/drivers/hsi/clients/hsi_ffl_tty.c @@ -895,6 +895,7 @@ static int _ffl_from_wait_to_ctrl(struct ffl_xfer_ctx *ctx, struct hsi_msg *frame, unsigned long *flags) { unsigned int actual_len = frame->actual_len; + struct ffl_ctx *main_ctx; int err; if (unlikely(_ffl_ctx_has_flag(ctx, TX_TTY_WRITE_FORWARDING_BIT))) @@ -909,9 +910,12 @@ static int _ffl_from_wait_to_ctrl(struct ffl_xfer_ctx *ctx, /* Keep the data for the future */ _ffl_fifo_wait_push_back(ctx, frame); } else { - if (ctx->ctrl_len > 0) - mod_timer(&ctx->main_ctx->hangup.timer, + if (ctx->ctrl_len > 0) { + main_ctx = container_of(ctx, struct ffl_ctx, tx); + mod_timer(&main_ctx->hangup.timer, jiffies + usecs_to_jiffies(TTY_HANGUP_DELAY)); + del_timer(&ctx->timer); + } #ifdef CONFIG_HSI_FFL_TTY_STATS ctx->data_sz += actual_len; ctx->frame_cnt++; @@ -922,6 +926,24 @@ static int _ffl_from_wait_to_ctrl(struct ffl_xfer_ctx *ctx, } /** + * _ffl_update_state_tx - update the tx state and timers + * @ctx: a reference to the FFL TX context to consider + */ +static void _ffl_update_state_tx(struct ffl_xfer_ctx *ctx) +{ + struct ffl_ctx *main_ctx = container_of(ctx, struct ffl_ctx, tx); + + if ((ctx->ctrl_len <= 0) && + (likely(!_ffl_ctx_has_flag(ctx, TX_TTY_WRITE_FORWARDING_BIT)))) { + del_timer(&main_ctx->hangup.timer); + if (ctx->wait_len == 0) { + wake_up(&main_ctx->tx_full_pipe_clean_event); + mod_timer(&ctx->timer, jiffies + ctx->delay); + } + } +} + +/** * _ffl_pop_wait_push_ctrl - transfer as many TX frames as possible from the * wait FIFO to the controller FIFO, unless a frame * is being updated (marked as not completed) @@ -943,6 +965,8 @@ static void _ffl_pop_wait_push_ctrl(struct ffl_xfer_ctx *ctx, (_ffl_from_wait_to_ctrl(ctx, frame, flags) != 0)) break; } + + _ffl_update_state_tx(ctx); } /* @@ -1073,16 +1097,6 @@ static __must_check inline int _ffl_ctx_is_empty(struct ffl_xfer_ctx *ctx) } /** - * _ffl_tx_ctx_is_empty - checks if a TX context is empty (all FIFO are empty) - * @ctx: a reference to the TX FFL context to consider - */ -static __must_check inline int _ffl_tx_ctx_is_empty(struct ffl_xfer_ctx *ctx) -{ - return _ffl_ctx_is_empty(ctx) && - (likely(!_ffl_ctx_has_flag(ctx, TX_TTY_WRITE_FORWARDING_BIT))); -} - -/** * ffl_tx_full_pipe_is_clean - checks if the TX pipe is clean or about to be * @ctx: a reference to the main FFL context to consider * @@ -1097,7 +1111,10 @@ static __must_check int ffl_tx_full_pipe_is_clean(struct ffl_ctx *ctx) unsigned long flags; spin_lock_irqsave(&tx_ctx->lock, flags); - ret = _ffl_tx_ctx_is_empty(tx_ctx) || (ctx->hangup.cause); + ret = (_ffl_ctx_is_empty(tx_ctx) && + (likely(!_ffl_ctx_has_flag(tx_ctx, + TX_TTY_WRITE_FORWARDING_BIT)))) || + (ctx->hangup.cause); spin_unlock_irqrestore(&tx_ctx->lock, flags); return ret; @@ -1405,16 +1422,10 @@ static void ffl_destruct_frame(struct hsi_msg *frame) spin_lock_irqsave(&ctx->lock, flags); _ffl_fifo_ctrl_pop(ctx); _ffl_free_frame(ctx, frame); - if (xfer_ctx_is_rx_ctx(ctx)) { + if (xfer_ctx_is_rx_ctx(ctx)) _ffl_update_state_rx(ctx); - } else { - if (ctx->ctrl_len <= 0) - del_timer(&ctx->main_ctx->hangup.timer); - if (_ffl_tx_ctx_is_empty(ctx)) { - wake_up(&ctx->main_ctx->tx_full_pipe_clean_event); - mod_timer(&ctx->timer, jiffies + ctx->delay); - } - } + else + _ffl_update_state_tx(ctx); spin_unlock_irqrestore(&ctx->lock, flags); } @@ -1573,19 +1584,8 @@ static void ffl_complete_tx(struct hsi_msg *frame) main_ctx->reset.ongoing = 0; spin_lock_irqsave(&ctx->lock, flags); - _ffl_free_frame(ctx, frame); _ffl_fifo_ctrl_pop(ctx); - if (ctx->ctrl_len > 0) - mod_timer(&main_ctx->hangup.timer, - jiffies + usecs_to_jiffies(TTY_HANGUP_DELAY)); - else - del_timer(&main_ctx->hangup.timer); - if (_ffl_tx_ctx_is_empty(ctx)) { - wake_up(&main_ctx->tx_full_pipe_clean_event); - mod_timer(&ctx->timer, jiffies + ctx->delay); - } else { - del_timer(&ctx->timer); - } + _ffl_free_frame(ctx, frame); /* Start new waiting frames (if any) */ _ffl_pop_wait_push_ctrl(ctx, &flags); @@ -1879,7 +1879,7 @@ static void ffl_rx_forward_resume(struct tty_struct *tty) /** * from_usecs - translating usecs to jiffies - * @delay: the dealy in usecs + * @delay: the delay in usecs * * Returns the delay rounded up to the next jiffy and prevent it to be set * to zero, as all delayed function calls shall occur to the next jiffy (at @@ -2111,38 +2111,29 @@ static void ffl_flush_tx_buffer(struct tty_struct *tty) } /** - * ffl_throw_tty_hangup - throwing a work for TTY hangup request - * @ctx: a reference to the main FFL context - * @cause: the cause for the TTY hangup request + * ffl_tty_tx_timeout - timer function for tx timeout hangup request + * @param: a hidden reference to the main FFL context */ -static void ffl_throw_tty_hangup(struct ffl_ctx *ctx, int cause) +static void ffl_tty_tx_timeout(unsigned long int param) { + struct ffl_ctx *ctx = (struct ffl_ctx *)param; struct ffl_xfer_ctx *tx_ctx = &ctx->tx; unsigned long flags; - int do_hangup; + int do_hangup = 0; spin_lock_irqsave(&tx_ctx->lock, flags); - do_hangup = ((!_ffl_ctx_has_flag(tx_ctx, TTY_OFF_BIT)) && - (!ctx->hangup.cause)); - ctx->hangup.cause |= cause; - wake_up(&ctx->tx_full_pipe_clean_event); + if (likely(tx_ctx->ctrl_len > 0)) { + do_hangup = ((!_ffl_ctx_has_flag(tx_ctx, TTY_OFF_BIT)) && + (!ctx->hangup.cause)); + ctx->hangup.cause |= HU_TIMEOUT; + wake_up(&ctx->tx_full_pipe_clean_event); + } spin_unlock_irqrestore(&tx_ctx->lock, flags); - if (do_hangup) + if (do_hangup) { + pr_err(DRVNAME ": TX timeout"); queue_work(ffl_hangup_wq, &ctx->hangup.work); -} - -/** - * ffl_tty_tx_timeout - timer function for tx timeout hangup request - * @param: a hidden reference to the main FFL context - */ -static void ffl_tty_tx_timeout(unsigned long int param) -{ - struct ffl_ctx *ctx = (struct ffl_ctx *)param; - - pr_err(DRVNAME ": TX timeout"); - - ffl_throw_tty_hangup(ctx, HU_TIMEOUT); + } } /** @@ -2855,7 +2846,7 @@ static irqreturn_t ffl_reset_isr(int irq, void *dev) struct ffl_xfer_ctx *tx_ctx; struct hsi_client *client; struct hsi_mid_platform_data *pd = NULL; - int status, cause = 0; + int do_hangup, status, cause = 0; ctx = (struct ffl_ctx *)dev; tx_ctx = &ctx->tx; @@ -2867,21 +2858,32 @@ static irqreturn_t ffl_reset_isr(int irq, void *dev) if (irq == ctx->reset.irq) { status = gpio_get_value(pd->gpio_mdm_rst_out); - dev_dbg(&client->device, "GPIO RESET_OUT %x", status); /* Prevent issuing hang-up for the usual reset toggling whilst * the modem is resetting */ cause = (ctx->reset.ongoing) ? 0 : HU_RESET; + dev_dbg(&client->device, "GPIO RESET_OUT %x (%d)", status, + cause); } else if (irq == ctx->reset.cd_irq) { status = gpio_get_value(pd->gpio_fcdp_rb); - dev_dbg(&client->device, "GPIO CORE_DUMP %x", status); /* Skip fake core dump sequences */ cause = (ctx->reset.ongoing) ? 0 : HU_COREDUMP; + dev_dbg(&client->device, "GPIO CORE_DUMP %x (%d)", status, + cause); } if (cause) { if (cause == HU_RESET) ctx->reset.ongoing = 1; - ffl_throw_tty_hangup(ctx, cause); + + spin_lock(&tx_ctx->lock); + do_hangup = ((!_ffl_ctx_has_flag(tx_ctx, TTY_OFF_BIT)) && + (!ctx->hangup.cause)); + ctx->hangup.cause |= cause; + wake_up(&ctx->tx_full_pipe_clean_event); + spin_unlock(&tx_ctx->lock); + + if (do_hangup) + queue_work(ffl_hangup_wq, &ctx->hangup.work); } return IRQ_HANDLED; -- 2.7.4