public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Hurley <peter@hurleysoftware.com>
To: "Matwey V. Kornilov" <matwey@sai.msu.ru>,
	One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>,
	Greg KH <gregkh@linuxfoundation.org>
Cc: jslaby@suse.com, linux-kernel <linux-kernel@vger.kernel.org>,
	linux-serial@vger.kernel.org
Subject: Re: [PATCH v3 2/5] tty: Introduce SER_RS485_SOFTWARE read-only flag for struct serial_rs485
Date: Wed, 2 Dec 2015 18:20:02 -0500	[thread overview]
Message-ID: <565F7CA2.5030505@hurleysoftware.com> (raw)
In-Reply-To: <CAJs94EY=qAhn6dzH9JbREruoXrvZoJh6a4jZEFHrK_KGGAre8g@mail.gmail.com>

On 11/18/2015 02:49 PM, Matwey V. Kornilov wrote:
> 2015-11-18 22:39 GMT+03:00 Matwey V. Kornilov <matwey@sai.msu.ru>:
>> 2015-11-18 21:33 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>>> On 11/17/2015 03:20 AM, Matwey V. Kornilov wrote:
>>>> 2015-11-16 22:18 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>>>>> On 11/14/2015 10:25 AM, One Thousand Gnomes wrote:
[...]
>>>>>> It's also not "easy to drop". If it ever goes in we are stuck with a
>>>>>> pointless impossible to correctly set flag for all eternity.
>>>>>>
>>>>>> Please explain the correct setting for this flag when a device driver
>>>>>> uses hardware or software or a mix according to what the silicon is
>>>>>> capable of and what values are requested ? How will an application use the
>>>>>> flag meaningfully. Please explain what will happen if someone discovers a
>>>>>> silicon bug and in a future 4.x release turns an implementation from
>>>>>> hardware to software - will they have to lie about the flag to avoid
>>>>>> breaking their application code - that strikes me as a bad thing.
>>>>>
>>>>> The existing driver behavior is already significantly variant and needs
>>>>> to be converged, which shouldn't be too difficult. Here's a quick summary:
>>>>>
>>>>> mcfuart         ignores delay values, delays unsupported
>>>>> imx             clamps delay values to 0, delays unsupported
>>>>> atmel           only delay_rts_after_send used; delay_rts_before_send does nothing
>>>>> 8250_fintek     clamps delay values to 1, unclear if h/w delay is msecs
>>>>> omap-serial*    software emulation (but tx empty polling not reqd)
>>>>> lpc18xx-uart    clamps delay_rts_before_send to 0, unsupported
>>>>>                 clamps delay_rts_after_send to max h/w value
>>>>> max310x         returns -ERANGE if either delay value > h/w support (15 msecs)
>>>>> sc16is7xx*      returns -EINVAL if delay_rts_after_send is set
>>>>> crisv10*        clamps delay_rts_before_send to 1000 msecs
>>>>>                 ignores delays_rts_after_send (after dma is delayed by 2 * chars)
>>>>> * implements delay(s) in software
>>>>>
>>>>> The omap-serial emulation should not have been merged in its current form.
>>>>>
>>>>> IMO the proper driver behavior should be clamp to h/w limit so an application
>>>>> can determine the maximum delay supported. If a delay is unsupported, it should
>>>>> be clamped to 0. The application should check the RS485 settings returned by
>>>>> TIOCSRS485 to determine how the driver set them.
>>>>> [ Documentation/serial/serial-rs485.txt should suggest/model this action ]
>>>>
>>>> But the similar could be true for minimal supported delay. If user
>>>> requires delay which is less than lower bound, the delay is raised to
>>>> the lower bound. If user requires delay which is greater than upper
>>>> bound, the delay is set to the upper bound. Then software
>>>> implementation could use (tx fifo size / baudrate) as lower bound for
>>>> delay_after_send.
>>>
>>> From the application point-of-view (really the only relevant semantics),
>>> delay_dts_after_send refers to the number of milliseconds to delay the
>>> toggle of RTS after the last bit has been _transmitted_.

Is there consensus then about what the semantics of unsupported RS485 delay
values are? I (or someone else) can trivially add the documentation and
fixes to the existing in-tree drivers.


>>> A couple of possibilities for improving the emulation are:
>>> 1) Optionally using an HR timer for sub-jiffy turnaround.
>>> 2) Only supporting 8250-based hardware that can be set to interrupt when
>>>    both tx fifo and transmitter shift register are empty.
>>
>> This is to support the RS485 API with already exists in omap_serrial,
>> but not in 8250_omap. And OMAP does not support tx line interrupt in
>> UART mode. So the latter is not an option.
> 
> Oh, I am sorry, it does support. There is "Supplementary Control
> Register" described in 19.5.1.39

For the moment then, can we add a UART_CAP_SW485 (not exposed to userspace)
that enables this algorithm only for h/w that supports a both-empty interrupt
mode. The probe or driver (ala 8250_omap) would opt-in and configure the h/w much
like the omap-serial driver does now (with the SCR register).

Does that seem like an acceptable compromise?

Regards,
Peter Hurley

PS - I still need to review this series for how the timer logic works esp. wrt
teardown.


  reply	other threads:[~2015-12-02 23:20 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-12 14:33 [PATCH v3 0/5] tty: Introduce software RS485 direction control support Matwey V. Kornilov
2015-11-12 14:33 ` [PATCH v3 1/5] tty: Introduce UART_CAP_HW485 Matwey V. Kornilov
2015-11-12 14:33 ` [PATCH v3 2/5] tty: Introduce SER_RS485_SOFTWARE read-only flag for struct serial_rs485 Matwey V. Kornilov
2015-11-12 19:57   ` One Thousand Gnomes
2015-11-12 20:22     ` Peter Hurley
2015-11-13  0:41       ` Andy Shevchenko
2015-11-13  1:11         ` Peter Hurley
2015-11-13  1:26           ` Andy Shevchenko
2015-11-13  1:55             ` Peter Hurley
2015-11-14 15:25       ` One Thousand Gnomes
2015-11-16 19:18         ` Peter Hurley
2015-11-17  8:20           ` Matwey V. Kornilov
2015-11-18 18:33             ` Peter Hurley
2015-11-18 19:39               ` Matwey V. Kornilov
2015-11-18 19:49                 ` Matwey V. Kornilov
2015-12-02 23:20                   ` Peter Hurley [this message]
2015-12-03  5:50                     ` Matwey V. Kornilov
2015-12-03 14:41                       ` Peter Hurley
2015-12-03 17:29                         ` Matwey V. Kornilov
2015-12-03 19:45                           ` Peter Hurley
2015-12-04 17:50                             ` Matwey V. Kornilov
2015-11-13 20:03     ` Matwey V. Kornilov
2015-11-12 14:33 ` [PATCH v3 3/5] tty: Implement default fallback serial8250_rs485_config Matwey V. Kornilov
2015-11-12 14:33 ` [PATCH v3 4/5] tty: Move serial8250_stop_rx in front of serial8250_start_tx Matwey V. Kornilov
2015-11-12 14:33 ` [PATCH v3 5/5] tty: Add software emulated RS485 support for 8250 Matwey V. Kornilov
2015-11-12 14:48   ` Andy Shevchenko
2015-11-17  9:24   ` Uwe Kleine-König
2015-11-17 10:25     ` Matwey V. Kornilov

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=565F7CA2.5030505@hurleysoftware.com \
    --to=peter@hurleysoftware.com \
    --cc=gnomes@lxorguk.ukuu.org.uk \
    --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