All of lore.kernel.org
 help / color / mirror / Atom feed
From: One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>
To: Peter Hurley <peter@hurleysoftware.com>
Cc: "Matwey V. Kornilov" <matwey@sai.msu.ru>,
	gregkh@linuxfoundation.org, jslaby@suse.com,
	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: Sat, 14 Nov 2015 15:25:36 +0000	[thread overview]
Message-ID: <20151114152536.693d964c@lxorguk.ukuu.org.uk> (raw)
In-Reply-To: <5644F4F2.2080408@hurleysoftware.com>

> 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

  parent reply	other threads:[~2015-11-14 15:26 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 [this message]
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
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=20151114152536.693d964c@lxorguk.ukuu.org.uk \
    --to=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 \
    --cc=peter@hurleysoftware.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.