unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman via Libc-alpha <libc-alpha@sourceware.org>
To: "Pali Rohár" <pali@kernel.org>
Cc: "Marek Behún" <kabel@kernel.org>,
	"Alejandro Colomar" <alx.manpages@gmail.com>,
	libc-alpha@sourceware.org, linux-man@vger.kernel.org,
	"Michael Kerrisk" <mtk.manpages@gmail.com>,
	linux-serial@vger.kernel.org,
	"G. Branden Robinson" <g.branden.robinson@gmail.com>
Subject: Re: [PATCH v3] ioctl_tty.2: Add example how to get or set baudrate on the serial port
Date: Thu, 5 Aug 2021 10:30:15 +0200	[thread overview]
Message-ID: <YQuhl18CgJ2+LUPW@kroah.com> (raw)
In-Reply-To: <20210805082243.qciylqnt5g74if7i@pali>

On Thu, Aug 05, 2021 at 10:22:43AM +0200, Pali Rohár wrote:
> On Thursday 05 August 2021 07:52:03 Greg Kroah-Hartman wrote:
> > On Thu, Aug 05, 2021 at 12:08:08AM +0200, Pali Rohár wrote:
> > > + linux-serial
> > > + Greg
> > > 
> > > Greg, could I ask you for reviewing this documentation manpage patch?
> > 
> > If it is submitted in a format I can review, sure (i.e. not top-post...)
> > 
> > But I will dig down below to say one thing...
> > 
> > > 
> > > On Sunday 01 August 2021 15:51:45 Pali Rohár wrote:
> > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > 
> > > > ---
> > > > Changes in v3:
> > > > * Check support for custom baudrate only based on BOTHER macro
> > > > * Use TCGETS/TCSETS/termios when TCGETS2/TCSETS2/termios2 is not available
> > > > 
> > > > Changes in v2:
> > > > * Use \e for backslash
> > > > * Use exit(EXIT_*) instead of return num
> > > > * Sort includes
> > > > * Add comment about possible fallback
> > > > ---
> > > > 
> > > > Hello Alejandro!
> > > > 
> > > > I found out that this stuff is more complicated as I originally thought.
> > > > And seems that additional documentation on this topic is needed...
> > > > 
> > > > For setting custom baudrate it is needed to set BOTHER flag in c_cflag
> > > > field and baudrate value itself in c_ospeed and c_ispeed fields.
> > > > 
> > > > So when BOTHER flag is not provided by <asm/termbits.h> then setting custom
> > > > baudrate is not possible, fields c_ospeed and c_ispeed do not exist (and
> > > > only some predefined Bnnn baudrate values are supported). This applies when
> > > > compiling application with older version of header files (prior support for
> > > > custom baudrate was introduced into header files).
> > > > 
> > > > First caveat: BOTHER constant is different for different architectures.
> > > > So it is not possible to provide fallback #ifndef..#define BOTHER.
> > > > 
> > > > And now the biggest issue: Some architectures have these c_ospeed and
> > > > c_ispeed fields in struct termios and some in struct termios2.
> > > > 
> > > > TCGETS/TCSETS ioctls use struct termios and TCGETS/TCSETS2 use
> > > > struct termios2.
> > > > 
> > > > Some architectures (e.g. amd64) provide both struct termios and struct
> > > > termios2, but c_ospeed and c_ispeed are only in struct termios2.
> > > > 
> > > > Some other architectures (e.g. alpha) provide both struct termios and struct
> > > > termios2 and both have c_ospeed and c_ispeed fields.
> > > > 
> > > > And some other architectures (e.g. powerpc) provide only struct termios
> > > > (no struct termios2) and it has c_ospeed and c_ispeed fields.
> > > > 
> > > > So basically to support all architectures it is needed to use
> > > > struct termios2 when TCGETS2/TCSETS2 is supported. Otherwise it is needed
> > > > to use struct termios with TCGETS/TCSETS (case for e.g. powerpc).
> > > > 
> > > > I updated v3 patch to handle this logic.
> > > > ---
> > > >  man2/ioctl_tty.2 | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 73 insertions(+)
> > > > 
> > > > diff --git a/man2/ioctl_tty.2 b/man2/ioctl_tty.2
> > > > index 3020f9984872..d83cbd17225b 100644
> > > > --- a/man2/ioctl_tty.2
> > > > +++ b/man2/ioctl_tty.2
> > > > @@ -764,6 +764,79 @@ main(void)
> > > >      close(fd);
> > > >  }
> > > >  .EE
> > > > +.PP
> > > > +Get or set arbitrary baudrate on the serial port.
> > > > +.PP
> > > > +.EX
> > > > +#include <asm/termbits.h>
> > > > +#include <fcntl.h>
> > > > +#include <stdio.h>
> > > > +#include <stdlib.h>
> > > > +#include <sys/ioctl.h>
> > > > +#include <sys/types.h>
> > > > +#include <unistd.h>
> > > > +
> > > > +int
> > > > +main(int argc, char *argv[])
> > > > +{
> > > > +#ifndef BOTHER
> > > > +    fprintf(stderr, "BOTHER is unsupported\en");
> > > > +    /* Program may fallback to TCGETS/TCSETS with Bnnn constants */
> > > > +    exit(EXIT_FAILURE);
> > 
> > So this is a BOTHER test only?
> 
> Yes.
> 
> > What is the goal of this program?  Don't throw a bunch of #ifdef in here
> > for no good reason.  These options should all be present on all normal
> > kernels, why wouldn't they be?
> 
> I wanted to provide complete example which compiles fine on all Linux
> systems, even with older include header files. I do not know right now
> in which kernel version was introduced BOTHER support for all
> architectures.

We have all of the kernel source in a tool that would allow you to to
determine this quite easily :)

> If BOHTER is not supported then it is possible to still use Bnnn
> constants to get / set baudrate. Just it is needed to write long code
> for converting number to suitable Bnnn constant.
> 
> Do you think that this BOTHER check is not useful in this case?

I think you should provide an example of how to use BOTHER, yes, as it
is hard to find good examples out there as they keep floating around.

Here's one that I point people to a lot:
	https://github.com/GrantEdwards/Linux-arbitrary-baud

Make the example code easy to follow.

Also, you forgot a license for this code, that is required if you want
people to use it...

thanks,

greg k-h

  reply	other threads:[~2021-08-05  8:31 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-30  9:53 [PATCH] ioctl_tty.2: Add example how to get or set baudrate on the serial port Pali Rohár via Libc-alpha
2021-07-30 11:47 ` Alejandro Colomar (man-pages) via Libc-alpha
2021-07-30 12:05   ` Pali Rohár via Libc-alpha
2021-07-30 12:21     ` Alejandro Colomar (man-pages) via Libc-alpha
2021-07-30 13:02 ` [PATCH v2] " Pali Rohár via Libc-alpha
2021-08-01 13:51 ` [PATCH v3] " Pali Rohár via Libc-alpha
2021-08-04 22:08   ` Pali Rohár via Libc-alpha
2021-08-05  5:52     ` Greg Kroah-Hartman via Libc-alpha
2021-08-05  8:22       ` Pali Rohár via Libc-alpha
2021-08-05  8:30         ` Greg Kroah-Hartman via Libc-alpha [this message]
2021-08-05  8:44           ` Pali Rohár via Libc-alpha
2021-08-05  8:50             ` Greg Kroah-Hartman via Libc-alpha
2021-08-05  9:51               ` Pali Rohár via Libc-alpha
2021-08-05 15:28                 ` Alejandro Colomar (man-pages) via Libc-alpha
2021-08-05 16:14                   ` Greg Kroah-Hartman via Libc-alpha
2021-08-05 16:45                     ` Licensing example programs in man-pages (was [PATCH v3] ioctl_tty.2: Add example how to get or set baudrate on the serial port) Alejandro Colomar (man-pages) via Libc-alpha
2021-08-05 17:54                       ` Greg Kroah-Hartman via Libc-alpha
2021-08-06  7:22                         ` Alejandro Colomar (man-pages) via Libc-alpha
2021-08-06  8:32                           ` Pali Rohár via Libc-alpha
2021-08-08  8:35   ` [PATCH v3] ioctl_tty.2: Add example how to get or set baudrate on the serial port Alejandro Colomar (man-pages) via Libc-alpha
2021-08-08 21:05     ` Pali Rohár via Libc-alpha
2021-08-08 21:19       ` Alejandro Colomar (man-pages) via Libc-alpha
2021-08-10 19:49 ` [PATCH v4] " Pali Rohár via Libc-alpha
2021-08-10 20:11   ` Pali Rohár via Libc-alpha
2021-08-31 20:34   ` Pali Rohár via Libc-alpha
2021-09-10 13:37     ` Alejandro Colomar (man-pages) via Libc-alpha
2021-09-10 13:39       ` Pali Rohár via Libc-alpha

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

  List information: https://www.gnu.org/software/libc/involved.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YQuhl18CgJ2+LUPW@kroah.com \
    --to=libc-alpha@sourceware.org \
    --cc=alx.manpages@gmail.com \
    --cc=g.branden.robinson@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kabel@kernel.org \
    --cc=linux-man@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=mtk.manpages@gmail.com \
    --cc=pali@kernel.org \
    /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 read-only IMAP folder(s) and NNTP newsgroup(s).