Re: [Xenomai] [PATCH] 16550: Fix crossed timeouts in rt_16550_write

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2013-01-28 20:53, Doug Brunner wrote:
> rt_16550_write currently initializes its timeout sequence using
> rx_timeout. Because xnsynch_acquire ignores the value of timeout (apart
> from making sure it's positive) when it is passed a non NULL timeout
> sequence, this means that serial writes are using the read timeout,
> except for when the user sets a zero tx timeout.

I would make the description shorter: We accidentally use rx_timeout
instead of tx_timeout in parts of rt_dev_write. This fixes the obvious
copy&paste mistake.

> (Should xnsynch_acquire
> be changed to (timeout + current time) to *timeout_seq to decide which
> will lead to timing out sooner? Seems more consistent but might break
> existing code...)

That's controlled by timeout_mode (XN_RELATIVE vs. XN_ABSOLUTE).

> 
> This changes the initialization of the timeout sequence to use
> tx_timeout. I also changed the first call to rtdm_mutex_timedlock to use
> tx_timeout, since this more accurately reflects what is happening.
> 
> *******************

Use "---" as separator - or git format-patch.

> 
> diff --git a/ksrc/drivers/serial/16550A.c b/ksrc/drivers/serial/16550A.c
> index 3672539..e92017d 100644
> --- a/ksrc/drivers/serial/16550A.c
> +++ b/ksrc/drivers/serial/16550A.c
> @@ -979,10 +979,10 @@ ssize_t rt_16550_write(struct rtdm_dev_context *
> context,
> 
>         ctx = (struct rt_16550_context *)context->dev_private;
> 
> -       rtdm_toseq_init(&timeout_seq, ctx->config.rx_timeout);
> +       rtdm_toseq_init(&timeout_seq, ctx->config.tx_timeout);
> 
>         /* Make write operation atomic. */
> -       ret = rtdm_mutex_timedlock(&ctx->out_lock, ctx->config.rx_timeout,
> +       ret = rtdm_mutex_timedlock(&ctx->out_lock, ctx->config.tx_timeout,
>                                    &timeout_seq);
>         if (ret)
>                 return ret;
> 
> *******************

This line may confuse "git am" while applying, but I didn't test.

Anyway, the patch content is fine.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux