Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1270491 > unrolled thread

Re: [PATCH v3 2/5] tty: Introduce SER_RS485_SOFTWARE read-only flag for struct serial_rs485

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v3 2/5] tty: Introduce SER_RS485_SOFTWARE read-only flag  for struct serial_rs485 Peter Hurley <peter@hurleysoftware.com> - 2015-11-16 20:20 +0100
    Re: [PATCH v3 2/5] tty: Introduce SER_RS485_SOFTWARE read-only flag  for struct serial_rs485 "Matwey V. Kornilov" <matwey@sai.msu.ru> - 2015-11-17 09:30 +0100
      Re: [PATCH v3 2/5] tty: Introduce SER_RS485_SOFTWARE read-only flag  for struct serial_rs485 Peter Hurley <peter@hurleysoftware.com> - 2015-11-18 19:40 +0100
        Re: [PATCH v3 2/5] tty: Introduce SER_RS485_SOFTWARE read-only flag  for struct serial_rs485 "Matwey V. Kornilov" <matwey@sai.msu.ru> - 2015-11-18 20:50 +0100
          Re: [PATCH v3 2/5] tty: Introduce SER_RS485_SOFTWARE read-only flag  for struct serial_rs485 "Matwey V. Kornilov" <matwey@sai.msu.ru> - 2015-11-18 21:00 +0100

#1270491 — Re: [PATCH v3 2/5] tty: Introduce SER_RS485_SOFTWARE read-only flag for struct serial_rs485

On 11/14/2015 10:25 AM, One Thousand Gnomes wrote:
>> I specifically asked for it.
>>
>> I can think of 2 reasons that userspace wants to know:
>> 1. Because the characteristics of the software emulation are unacceptable so
>>    the application wants to terminate w/error rather than continue.
> 
> But that could equally be true of hardware.

I had this exact same thought, but concluded that it argues for a way
to select the software implementation even when h/w supports RS485.

> In fact your software
> emulation is going to behave vastly better than many of the hardware ones.
> 
>> 2. Because userspace will use different values for h/w vs. s/w. For example,
>>    right now, the emulation will raise/lower RTS prematurely when tx ends if
>>    the rts-after-send timer is 0.
> 
> That's a bug then. It should be fixed as part of the merge or future
> patches - if they are not providing that emulation then they ought to do
> so and at least adjust the timing based on the baud rate so you don't
> have to spin polling the 16x50 uart to check the last bit fell out of the
> register.

I suppose the timer(s) could be fudged and then TEMT polled (or polled every
char baud cycles). But I don't see how this will behave better than a h/w
implementation; the granularity of the jiffy clock alone will guarantee
sub-optimal turnaround, even at 9600.

> I'd have no problem with an API that was about asking what features are
> available : both hardware and software - but the software flag seems to
> make no sense at all. Software doesn't imply anything about quality or
> feature set. If there is something the emulation cannot support then
> there should be a flag indicating that feature is not supported, not a
> flag saying software (which means nothing - as it may be supported in
> future, or may differ by uart etc).

Fair enough.

> 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 ]

Are TIOCGRS485 and TIOCSRS485 documented in tty_ioctl man page? (I haven't
updated my man pages in a while)

As far as software vs. hardware and a query api, what I care about is
conveying to userspace whether the implementation will be adequate for purpose,
with the main issue being the true delay from actual EOT to RTS toggle
when delay_after_rts_send == 0.

Since that delay is unbounded with software methods, I thought it made sense to
indicate that with a read-only bit. Naming it something else is fine too;
SER_RS485_NOT_REALTIME_EOT?

A more comprehensive approach might be to add a capabilities word
to struct serial_rs485 which would allow the driver to report what
it supports; eg. realtime turnaround or not, etc. (Not sure if extending
struct serial_rs485 is really possible; the serial core hasn't been
clearing padding on the driver's behalf).

> At the very least the above should be clearly explained in the
> documentation and patch covering notes - and if nobody can explain those
> then IMHO the flag is broken.

Yep.

Regards,
Peter Hurley

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1270948

2015-11-16 22:18 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
> On 11/14/2015 10:25 AM, One Thousand Gnomes wrote:
>>> I specifically asked for it.
>>>
>>> I can think of 2 reasons that userspace wants to know:
>>> 1. Because the characteristics of the software emulation are unacceptable so
>>>    the application wants to terminate w/error rather than continue.
>>
>> But that could equally be true of hardware.
>
> I had this exact same thought, but concluded that it argues for a way
> to select the software implementation even when h/w supports RS485.
>
>> In fact your software
>> emulation is going to behave vastly better than many of the hardware ones.
>>
>>> 2. Because userspace will use different values for h/w vs. s/w. For example,
>>>    right now, the emulation will raise/lower RTS prematurely when tx ends if
>>>    the rts-after-send timer is 0.
>>
>> That's a bug then. It should be fixed as part of the merge or future
>> patches - if they are not providing that emulation then they ought to do
>> so and at least adjust the timing based on the baud rate so you don't
>> have to spin polling the 16x50 uart to check the last bit fell out of the
>> register.
>
> I suppose the timer(s) could be fudged and then TEMT polled (or polled every
> char baud cycles). But I don't see how this will behave better than a h/w
> implementation; the granularity of the jiffy clock alone will guarantee
> sub-optimal turnaround, even at 9600.
>
>> I'd have no problem with an API that was about asking what features are
>> available : both hardware and software - but the software flag seems to
>> make no sense at all. Software doesn't imply anything about quality or
>> feature set. If there is something the emulation cannot support then
>> there should be a flag indicating that feature is not supported, not a
>> flag saying software (which means nothing - as it may be supported in
>> future, or may differ by uart etc).
>
> Fair enough.
>
>> 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.

>
> Are TIOCGRS485 and TIOCSRS485 documented in tty_ioctl man page? (I haven't
> updated my man pages in a while)
>
> As far as software vs. hardware and a query api, what I care about is
> conveying to userspace whether the implementation will be adequate for purpose,
> with the main issue being the true delay from actual EOT to RTS toggle
> when delay_after_rts_send == 0.

Or I just can internally add (tx fifo size / baudrate) to the user
supplied value to take care of the bytes in tx fifo.

>
> Since that delay is unbounded with software methods, I thought it made sense to
> indicate that with a read-only bit. Naming it something else is fine too;
> SER_RS485_NOT_REALTIME_EOT?
>
> A more comprehensive approach might be to add a capabilities word
> to struct serial_rs485 which would allow the driver to report what
> it supports; eg. realtime turnaround or not, etc. (Not sure if extending
> struct serial_rs485 is really possible; the serial core hasn't been
> clearing padding on the driver's behalf).
>
>> At the very least the above should be clearly explained in the
>> documentation and patch covering notes - and if nobody can explain those
>> then IMHO the flag is broken.
>
> Yep.
>
> Regards,
> Peter Hurley
>



-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1272465

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:
>>>> I specifically asked for it.
>>>>
>>>> I can think of 2 reasons that userspace wants to know:
>>>> 1. Because the characteristics of the software emulation are unacceptable so
>>>>    the application wants to terminate w/error rather than continue.
>>>
>>> But that could equally be true of hardware.
>>
>> I had this exact same thought, but concluded that it argues for a way
>> to select the software implementation even when h/w supports RS485.
>>
>>> In fact your software
>>> emulation is going to behave vastly better than many of the hardware ones.
>>>
>>>> 2. Because userspace will use different values for h/w vs. s/w. For example,
>>>>    right now, the emulation will raise/lower RTS prematurely when tx ends if
>>>>    the rts-after-send timer is 0.
>>>
>>> That's a bug then. It should be fixed as part of the merge or future
>>> patches - if they are not providing that emulation then they ought to do
>>> so and at least adjust the timing based on the baud rate so you don't
>>> have to spin polling the 16x50 uart to check the last bit fell out of the
>>> register.
>>
>> I suppose the timer(s) could be fudged and then TEMT polled (or polled every
>> char baud cycles). But I don't see how this will behave better than a h/w
>> implementation; the granularity of the jiffy clock alone will guarantee
>> sub-optimal turnaround, even at 9600.
>>
>>> I'd have no problem with an API that was about asking what features are
>>> available : both hardware and software - but the software flag seems to
>>> make no sense at all. Software doesn't imply anything about quality or
>>> feature set. If there is something the emulation cannot support then
>>> there should be a flag indicating that feature is not supported, not a
>>> flag saying software (which means nothing - as it may be supported in
>>> future, or may differ by uart etc).
>>
>> Fair enough.
>>
>>> 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_.

I agree with Alan that any adjustment to the delay to adhere to that
meaning needs to be transparent to user-space.


>> Are TIOCGRS485 and TIOCSRS485 documented in tty_ioctl man page? (I haven't
>> updated my man pages in a while)
>>
>> As far as software vs. hardware and a query api, what I care about is
>> conveying to userspace whether the implementation will be adequate for purpose,
>> with the main issue being the true delay from actual EOT to RTS toggle
>> when delay_after_rts_send == 0.
> 
> Or I just can internally add (tx fifo size / baudrate) to the user
> supplied value to take care of the bytes in tx fifo.

Yes. Or poll every jiffy.

But either will be far too coarse for many users; a delay_rts_after_send of
0 could still produce multi- _msec_ delays when the application expects
turnaround of ~1 char time. At a leisurely 19200 baud, that's ~520us which will
not be possible with this emulation.

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.

But regardless, the driver should still advertise whether direction control
is realtime or not (ie., software or not).

Regards,
Peter Hurley


>> Since that delay is unbounded with software methods, I thought it made sense to
>> indicate that with a read-only bit. Naming it something else is fine too;
>> SER_RS485_NOT_REALTIME_EOT?
>>
>> A more comprehensive approach might be to add a capabilities word
>> to struct serial_rs485 which would allow the driver to report what
>> it supports; eg. realtime turnaround or not, etc. (Not sure if extending
>> struct serial_rs485 is really possible; the serial core hasn't been
>> clearing padding on the driver's behalf).
>>
>>> At the very least the above should be clearly explained in the
>>> documentation and patch covering notes - and if nobody can explain those
>>> then IMHO the flag is broken.
>>
>> Yep.
>>
>> Regards,
>> Peter Hurley

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1272511

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:
>>>>> I specifically asked for it.
>>>>>
>>>>> I can think of 2 reasons that userspace wants to know:
>>>>> 1. Because the characteristics of the software emulation are unacceptable so
>>>>>    the application wants to terminate w/error rather than continue.
>>>>
>>>> But that could equally be true of hardware.
>>>
>>> I had this exact same thought, but concluded that it argues for a way
>>> to select the software implementation even when h/w supports RS485.
>>>
>>>> In fact your software
>>>> emulation is going to behave vastly better than many of the hardware ones.
>>>>
>>>>> 2. Because userspace will use different values for h/w vs. s/w. For example,
>>>>>    right now, the emulation will raise/lower RTS prematurely when tx ends if
>>>>>    the rts-after-send timer is 0.
>>>>
>>>> That's a bug then. It should be fixed as part of the merge or future
>>>> patches - if they are not providing that emulation then they ought to do
>>>> so and at least adjust the timing based on the baud rate so you don't
>>>> have to spin polling the 16x50 uart to check the last bit fell out of the
>>>> register.
>>>
>>> I suppose the timer(s) could be fudged and then TEMT polled (or polled every
>>> char baud cycles). But I don't see how this will behave better than a h/w
>>> implementation; the granularity of the jiffy clock alone will guarantee
>>> sub-optimal turnaround, even at 9600.
>>>
>>>> I'd have no problem with an API that was about asking what features are
>>>> available : both hardware and software - but the software flag seems to
>>>> make no sense at all. Software doesn't imply anything about quality or
>>>> feature set. If there is something the emulation cannot support then
>>>> there should be a flag indicating that feature is not supported, not a
>>>> flag saying software (which means nothing - as it may be supported in
>>>> future, or may differ by uart etc).
>>>
>>> Fair enough.
>>>
>>>> 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_.
>
> I agree with Alan that any adjustment to the delay to adhere to that
> meaning needs to be transparent to user-space.
>
>
>>> Are TIOCGRS485 and TIOCSRS485 documented in tty_ioctl man page? (I haven't
>>> updated my man pages in a while)
>>>
>>> As far as software vs. hardware and a query api, what I care about is
>>> conveying to userspace whether the implementation will be adequate for purpose,
>>> with the main issue being the true delay from actual EOT to RTS toggle
>>> when delay_after_rts_send == 0.
>>
>> Or I just can internally add (tx fifo size / baudrate) to the user
>> supplied value to take care of the bytes in tx fifo.
>
> Yes. Or poll every jiffy.
>
> But either will be far too coarse for many users; a delay_rts_after_send of
> 0 could still produce multi- _msec_ delays when the application expects
> turnaround of ~1 char time. At a leisurely 19200 baud, that's ~520us which will
> not be possible with this emulation.

If we want real-time, then we have to spin on LSR waiting for TXSRE be 1.

>
> 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.

>
> But regardless, the driver should still advertise whether direction control
> is realtime or not (ie., software or not).
>
> Regards,
> Peter Hurley
>
>
>>> Since that delay is unbounded with software methods, I thought it made sense to
>>> indicate that with a read-only bit. Naming it something else is fine too;
>>> SER_RS485_NOT_REALTIME_EOT?
>>>
>>> A more comprehensive approach might be to add a capabilities word
>>> to struct serial_rs485 which would allow the driver to report what
>>> it supports; eg. realtime turnaround or not, etc. (Not sure if extending
>>> struct serial_rs485 is really possible; the serial core hasn't been
>>> clearing padding on the driver's behalf).
>>>
>>>> At the very least the above should be clearly explained in the
>>>> documentation and patch covering notes - and if nobody can explain those
>>>> then IMHO the flag is broken.
>>>
>>> Yep.
>>>
>>> Regards,
>>> Peter Hurley
>



-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1272516

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:
>>>>>> I specifically asked for it.
>>>>>>
>>>>>> I can think of 2 reasons that userspace wants to know:
>>>>>> 1. Because the characteristics of the software emulation are unacceptable so
>>>>>>    the application wants to terminate w/error rather than continue.
>>>>>
>>>>> But that could equally be true of hardware.
>>>>
>>>> I had this exact same thought, but concluded that it argues for a way
>>>> to select the software implementation even when h/w supports RS485.
>>>>
>>>>> In fact your software
>>>>> emulation is going to behave vastly better than many of the hardware ones.
>>>>>
>>>>>> 2. Because userspace will use different values for h/w vs. s/w. For example,
>>>>>>    right now, the emulation will raise/lower RTS prematurely when tx ends if
>>>>>>    the rts-after-send timer is 0.
>>>>>
>>>>> That's a bug then. It should be fixed as part of the merge or future
>>>>> patches - if they are not providing that emulation then they ought to do
>>>>> so and at least adjust the timing based on the baud rate so you don't
>>>>> have to spin polling the 16x50 uart to check the last bit fell out of the
>>>>> register.
>>>>
>>>> I suppose the timer(s) could be fudged and then TEMT polled (or polled every
>>>> char baud cycles). But I don't see how this will behave better than a h/w
>>>> implementation; the granularity of the jiffy clock alone will guarantee
>>>> sub-optimal turnaround, even at 9600.
>>>>
>>>>> I'd have no problem with an API that was about asking what features are
>>>>> available : both hardware and software - but the software flag seems to
>>>>> make no sense at all. Software doesn't imply anything about quality or
>>>>> feature set. If there is something the emulation cannot support then
>>>>> there should be a flag indicating that feature is not supported, not a
>>>>> flag saying software (which means nothing - as it may be supported in
>>>>> future, or may differ by uart etc).
>>>>
>>>> Fair enough.
>>>>
>>>>> 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_.
>>
>> I agree with Alan that any adjustment to the delay to adhere to that
>> meaning needs to be transparent to user-space.
>>
>>
>>>> Are TIOCGRS485 and TIOCSRS485 documented in tty_ioctl man page? (I haven't
>>>> updated my man pages in a while)
>>>>
>>>> As far as software vs. hardware and a query api, what I care about is
>>>> conveying to userspace whether the implementation will be adequate for purpose,
>>>> with the main issue being the true delay from actual EOT to RTS toggle
>>>> when delay_after_rts_send == 0.
>>>
>>> Or I just can internally add (tx fifo size / baudrate) to the user
>>> supplied value to take care of the bytes in tx fifo.
>>
>> Yes. Or poll every jiffy.
>>
>> But either will be far too coarse for many users; a delay_rts_after_send of
>> 0 could still produce multi- _msec_ delays when the application expects
>> turnaround of ~1 char time. At a leisurely 19200 baud, that's ~520us which will
>> not be possible with this emulation.
>
> If we want real-time, then we have to spin on LSR waiting for TXSRE be 1.
>
>>
>> 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

>
>>
>> But regardless, the driver should still advertise whether direction control
>> is realtime or not (ie., software or not).
>>
>> Regards,
>> Peter Hurley
>>
>>
>>>> Since that delay is unbounded with software methods, I thought it made sense to
>>>> indicate that with a read-only bit. Naming it something else is fine too;
>>>> SER_RS485_NOT_REALTIME_EOT?
>>>>
>>>> A more comprehensive approach might be to add a capabilities word
>>>> to struct serial_rs485 which would allow the driver to report what
>>>> it supports; eg. realtime turnaround or not, etc. (Not sure if extending
>>>> struct serial_rs485 is really possible; the serial core hasn't been
>>>> clearing padding on the driver's behalf).
>>>>
>>>>> At the very least the above should be clearly explained in the
>>>>> documentation and patch covering notes - and if nobody can explain those
>>>>> then IMHO the flag is broken.
>>>>
>>>> Yep.
>>>>
>>>> Regards,
>>>> Peter Hurley
>>
>
>
>
> --
> With best regards,
> Matwey V. Kornilov.
> Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
> 119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382



-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web