From: Peter Hurley <peter@hurleysoftware.com>
To: "Matwey V. Kornilov" <matwey@sai.msu.ru>
Cc: Greg KH <gregkh@linuxfoundation.org>,
jslaby@suse.com, linux-serial@vger.kernel.org,
linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 3/3] tty: Add software emulated RS485 support for 8250
Date: Mon, 9 Nov 2015 17:05:10 -0500 [thread overview]
Message-ID: <56411896.6060801@hurleysoftware.com> (raw)
In-Reply-To: <CAJs94EZO6KP-3jWPAM-OqMc2rxekxojYbvssAjb3fvybS0-RtA@mail.gmail.com>
On 11/09/2015 04:43 PM, Matwey V. Kornilov wrote:
> 2015-11-10 0:30 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>> On 11/09/2015 10:45 AM, Matwey V. Kornilov wrote:
>>> 2015-11-09 17:40 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>>>> On 11/08/2015 05:52 AM, Matwey V. Kornilov wrote:
>>>>> 2015-11-07 19:03 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>>>>>> On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
>>
>> [...]
>>
>>>>>>> +static void serial8250_rs485_start_tx(struct uart_8250_port *p)
>>>>>>> +{
>>>>>>> + if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
>>>>>>> + return;
>>>>>>> +
>>>>>>> + if (p->port.rs485.flags & SER_RS485_RTS_ON_SEND) {
>>>>>>> + serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
>>>>>>> + if (p->port.rs485.delay_rts_before_send > 0)
>>>>>>> + mdelay(p->port.rs485.delay_rts_before_send);
>>>>>>
>>>>>> So irqs are off for x msecs, and this cpu can't be used for anything else now?
>>>>>> I think this needs to be solved differently; maybe with a timer?
>>>>>
>>>>> Call of serial8250_start_tx is wrapped with spin_lock_irq in serial_core.c:2154
>>>>
>>>> Yep, which is why I pointed out "irqs are off for x msecs".
>>>>
>>>>> I've tried to use msleep instead of mdelay but got "BUG: scheduling
>>>>> while atomic".
>>>>
>>>> Right, can't sleep while irqs are off, which is why I suggested something
>>>> like a timer.
>>>
>>> I am not sure that understand you correctly. Do you think that the
>>> following would be ok?
>>>
>>> wait_queue_head_t wait;
>>> init_waitqueue_head(&wait);
>>> wait_event_timeout(wait, 0, p->port.rs485.delay_rts_before_send * HZ / 1000);
>>
>> Except for spinning, there is no way to wait-in-place with irqs off.
>>
>> You'll need to do something more complex, like
>> 1. raise RTS
>> 2. start a timer _and return early without starting tx_
>> 3. timer goes off, handler actually starts tx
>>
>
> I think this could lead to race conditions.
> AFAIU when the kernel calls ops->start_tx(uport) and the function
> returns, then it is supposed that the tx has been started.
No; start_tx() must cause tx to become started, but tx does not
have to have _already_ started when start_tx() returns.
It would be very inefficient for start_tx() to _guarantee_ tx has
already started _before_ returning. Note the 8250 driver merely
writes to IER (which could be buffered and bridged).
> And that could be not true, if the timer is used.
It's true that using a timer will be more complex with more state
to manage, but being unable to service interrupts with this cpu for
milliseconds is unacceptable.
Regards,
Peter Hurley
next prev parent reply other threads:[~2015-11-09 22:05 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-07 10:09 [PATCH v2 1/3] tty: Introduce UART_CAP_HW485 Matwey V. Kornilov
2015-11-07 10:09 ` [PATCH v2 2/3] tty: Implement default fallback serial8250_rs485_config Matwey V. Kornilov
2015-11-07 12:29 ` Peter Hurley
2015-11-07 13:51 ` Matwey V. Kornilov
2015-11-07 14:20 ` Peter Hurley
2015-11-07 10:09 ` [PATCH v2 3/3] tty: Add software emulated RS485 support for 8250 Matwey V. Kornilov
2015-11-07 16:03 ` Peter Hurley
2015-11-08 10:52 ` Matwey V. Kornilov
2015-11-09 14:40 ` Peter Hurley
2015-11-09 15:45 ` Matwey V. Kornilov
2015-11-09 21:30 ` Peter Hurley
2015-11-09 21:43 ` Matwey V. Kornilov
2015-11-09 22:05 ` Peter Hurley [this message]
2015-11-10 11:35 ` Matwey V. Kornilov
2015-11-10 16:12 ` Peter Hurley
2015-11-10 16:25 ` Matwey V. Kornilov
2015-11-10 16:52 ` Peter Hurley
2015-11-12 12:34 ` Matwey V. Kornilov
2015-11-12 13:35 ` Peter Hurley
2015-11-13 8:35 ` Matwey V. Kornilov
2015-11-07 12:22 ` [PATCH v2 1/3] tty: Introduce UART_CAP_HW485 Peter Hurley
2015-11-07 12:39 ` Matwey V. Kornilov
2015-11-07 15:32 ` Peter Hurley
2015-11-07 21:10 ` Matwey V. Kornilov
2015-11-07 22:04 ` Peter Hurley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56411896.6060801@hurleysoftware.com \
--to=peter@hurleysoftware.com \
--cc=gregkh@linuxfoundation.org \
--cc=jslaby@suse.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=matwey@sai.msu.ru \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).