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

Groups > linux.kernel > #1267981 > unrolled thread

[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

  [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-12 15:40 +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-12 21:30 +0100
      Re: [PATCH v3 2/5] tty: Introduce SER_RS485_SOFTWARE read-only flag  for struct serial_rs485 Andy Shevchenko <andy.shevchenko@gmail.com> - 2015-11-13 01:50 +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-13 02:20 +0100
          Re: [PATCH v3 2/5] tty: Introduce SER_RS485_SOFTWARE read-only flag  for struct serial_rs485 Andy Shevchenko <andy.shevchenko@gmail.com> - 2015-11-13 02: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-13 03:00 +0100
      Re: [PATCH v3 2/5] tty: Introduce SER_RS485_SOFTWARE read-only flag  for struct serial_rs485 One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2015-11-14 16:30 +0100
    Re: [PATCH v3 2/5] tty: Introduce SER_RS485_SOFTWARE read-only flag  for struct serial_rs485 One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2015-11-12 21: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-13 21:10 +0100

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

This flag is supposed to be used by uart drivers using software rs485 direction control.

Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
---
 include/uapi/linux/serial.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
index 25331f9..95b15ca 100644
--- a/include/uapi/linux/serial.h
+++ b/include/uapi/linux/serial.h
@@ -121,6 +121,9 @@ struct serial_rs485 {
 #define SER_RS485_RTS_AFTER_SEND	(1 << 2)	/* Logical level for
 							   RTS pin after sent*/
 #define SER_RS485_RX_DURING_TX		(1 << 4)
+#define SER_RS485_SOFTWARE		(1 << 5)	/* Software
+							   implementation is
+							   being used */
 	__u32	delay_rts_before_send;	/* Delay before send (milliseconds) */
 	__u32	delay_rts_after_send;	/* Delay after send (milliseconds) */
 	__u32	padding[5];		/* Memory is cheap, new structs
-- 
2.6.2

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

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

On 11/12/2015 02:57 PM, One Thousand Gnomes wrote:
> On Thu, 12 Nov 2015 17:33:53 +0300
> "Matwey V. Kornilov" <matwey@sai.msu.ru> wrote:
> 
>> This flag is supposed to be used by uart drivers using software rs485 direction control.
>>
>> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
>> ---
>>  include/uapi/linux/serial.h | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
>> index 25331f9..95b15ca 100644
>> --- a/include/uapi/linux/serial.h
>> +++ b/include/uapi/linux/serial.h
>> @@ -121,6 +121,9 @@ struct serial_rs485 {
>>  #define SER_RS485_RTS_AFTER_SEND	(1 << 2)	/* Logical level for
>>  							   RTS pin after sent*/
>>  #define SER_RS485_RX_DURING_TX		(1 << 4)
>> +#define SER_RS485_SOFTWARE		(1 << 5)	/* Software
>> +							   implementation is
>> +							   being used */
> 
> I've only got one question here - why do we need this flag. Why does the
> application care whether the timer is in the kernel or in the chip. In
> particular think about cases where some combinations of features require
> software fallback and others don't. What would the flag indicate then.
> 
> The patches look nice but I'd strongly favour not having a software flag.
> It should never matter as the kernel API is the same in all cases and we
> should therefore discourage application code from trying to know things
> it doesn't need to worry about.

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

I agree that combination features might be problematic.
An illustrative (kernel-space) example is the mess that is dmaengine_pause().
Some DMA implementations provide the means to stop and restart DMA without
losing data and some DMA implementations do not. Unfortunately, some
advertise they support dmaengine_pause() but only for lossy uses like audio.
Because the api hides this, the query interface for pause support is
useless.

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]

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

On Thu, Nov 12, 2015 at 10:22 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
> On 11/12/2015 02:57 PM, One Thousand Gnomes wrote:

> An illustrative (kernel-space) example is the mess that is dmaengine_pause().
> Some DMA implementations provide the means to stop and restart DMA without
> losing data and some DMA implementations do not. Unfortunately, some
> advertise they support dmaengine_pause() but only for lossy uses like audio.
> Because the api hides this, the query interface for pause support is
> useless.

The DMA pause() call means only pause with possibility to resume.
There is a resume() call as well. Any driver which treats pause() as a
complete stop is buggy driver and should be fixed.

-- 
With Best Regards,
Andy Shevchenko
--
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]

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

On 11/12/2015 07:41 PM, Andy Shevchenko wrote:
> On Thu, Nov 12, 2015 at 10:22 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> On 11/12/2015 02:57 PM, One Thousand Gnomes wrote:
> 
>> An illustrative (kernel-space) example is the mess that is dmaengine_pause().
>> Some DMA implementations provide the means to stop and restart DMA without
>> losing data and some DMA implementations do not. Unfortunately, some
>> advertise they support dmaengine_pause() but only for lossy uses like audio.
>> Because the api hides this, the query interface for pause support is
>> useless.
> 
> The DMA pause() call means only pause with possibility to resume.
> There is a resume() call as well. Any driver which treats pause() as a
> complete stop is buggy driver and should be fixed.

How about pause _without_ the possibility to resume?

https://groups.google.com/d/msg/linux.kernel/Abe0hfGcgsw/H0se55wC558J

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

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

On Fri, Nov 13, 2015 at 3:11 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
> On 11/12/2015 07:41 PM, Andy Shevchenko wrote:
>> On Thu, Nov 12, 2015 at 10:22 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>> On 11/12/2015 02:57 PM, One Thousand Gnomes wrote:
>>
>>> An illustrative (kernel-space) example is the mess that is dmaengine_pause().
>>> Some DMA implementations provide the means to stop and restart DMA without
>>> losing data and some DMA implementations do not. Unfortunately, some
>>> advertise they support dmaengine_pause() but only for lossy uses like audio.
>>> Because the api hides this, the query interface for pause support is
>>> useless.
>>
>> The DMA pause() call means only pause with possibility to resume.
>> There is a resume() call as well. Any driver which treats pause() as a
>> complete stop is buggy driver and should be fixed.
>
> How about pause _without_ the possibility to resume?
>
> https://groups.google.com/d/msg/linux.kernel/Abe0hfGcgsw/H0se55wC558J

Briefly what I got from the thread that Russel shows similar view on
the API, so that's why he was objecting to add pause/resume calls for
a specific hardware.

-- 
With Best Regards,
Andy Shevchenko
--
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]

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

On 11/12/2015 08:26 PM, Andy Shevchenko wrote:
> On Fri, Nov 13, 2015 at 3:11 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> On 11/12/2015 07:41 PM, Andy Shevchenko wrote:
>>> On Thu, Nov 12, 2015 at 10:22 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>> On 11/12/2015 02:57 PM, One Thousand Gnomes wrote:
>>>
>>>> An illustrative (kernel-space) example is the mess that is dmaengine_pause().
>>>> Some DMA implementations provide the means to stop and restart DMA without
>>>> losing data and some DMA implementations do not. Unfortunately, some
>>>> advertise they support dmaengine_pause() but only for lossy uses like audio.
>>>> Because the api hides this, the query interface for pause support is
>>>> useless.
>>>
>>> The DMA pause() call means only pause with possibility to resume.
>>> There is a resume() call as well. Any driver which treats pause() as a
>>> complete stop is buggy driver and should be fixed.
>>
>> How about pause _without_ the possibility to resume?
>>
>> https://groups.google.com/d/msg/linux.kernel/Abe0hfGcgsw/H0se55wC558J
> 
> Briefly what I got from the thread that Russell shows similar view on
> the API, so that's why he was objecting to add pause/resume calls for
> a specific hardware.

Not quite.

That dmaengine driver (omap-dma) advertises that it supports pause() via
dma_get_slave_caps(). And if you call it with a cyclic channel it will pause.
However, if you call dmaengine_pause() with a slave channel it returns an error
*because the hardware can't actually meet the criteria for dmaengine_pause()*
which is pause()/resume() without data loss.

IOW, there is no method of determining a priori if dmaengine_pause() will
categorically fail for a given transfer type.

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]

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

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

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.

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.

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

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

On Thu, 12 Nov 2015 17:33:53 +0300
"Matwey V. Kornilov" <matwey@sai.msu.ru> wrote:

> This flag is supposed to be used by uart drivers using software rs485 direction control.
> 
> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
> ---
>  include/uapi/linux/serial.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
> index 25331f9..95b15ca 100644
> --- a/include/uapi/linux/serial.h
> +++ b/include/uapi/linux/serial.h
> @@ -121,6 +121,9 @@ struct serial_rs485 {
>  #define SER_RS485_RTS_AFTER_SEND	(1 << 2)	/* Logical level for
>  							   RTS pin after sent*/
>  #define SER_RS485_RX_DURING_TX		(1 << 4)
> +#define SER_RS485_SOFTWARE		(1 << 5)	/* Software
> +							   implementation is
> +							   being used */

I've only got one question here - why do we need this flag. Why does the
application care whether the timer is in the kernel or in the chip. In
particular think about cases where some combinations of features require
software fallback and others don't. What would the flag indicate then.

The patches look nice but I'd strongly favour not having a software flag.
It should never matter as the kernel API is the same in all cases and we
should therefore discourage application code from trying to know things
it doesn't need to worry about.

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

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

2015-11-12 22:57 GMT+03:00 One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>:
> On Thu, 12 Nov 2015 17:33:53 +0300
> "Matwey V. Kornilov" <matwey@sai.msu.ru> wrote:
>
>> This flag is supposed to be used by uart drivers using software rs485 direction control.
>>
>> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
>> ---
>>  include/uapi/linux/serial.h | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
>> index 25331f9..95b15ca 100644
>> --- a/include/uapi/linux/serial.h
>> +++ b/include/uapi/linux/serial.h
>> @@ -121,6 +121,9 @@ struct serial_rs485 {
>>  #define SER_RS485_RTS_AFTER_SEND     (1 << 2)        /* Logical level for
>>                                                          RTS pin after sent*/
>>  #define SER_RS485_RX_DURING_TX               (1 << 4)
>> +#define SER_RS485_SOFTWARE           (1 << 5)        /* Software
>> +                                                        implementation is
>> +                                                        being used */
>
> I've only got one question here - why do we need this flag. Why does the
> application care whether the timer is in the kernel or in the chip. In
> particular think about cases where some combinations of features require
> software fallback and others don't. What would the flag indicate then.
>

Peter asked for it, I respect his experience.
Only two lines are required to implement this, so it is easy to add,
easy to drop.

> The patches look nice but I'd strongly favour not having a software flag.
> It should never matter as the kernel API is the same in all cases and we
> should therefore discourage application code from trying to know things
> it doesn't need to worry about.
>
> Alan
>



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