From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS17314 8.43.84.0/22 X-Spam-Status: No, score=-4.2 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id DB00F1F8C6 for ; Thu, 5 Aug 2021 08:44:39 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 95C75386FC25 for ; Thu, 5 Aug 2021 08:44:38 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 95C75386FC25 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1628153078; bh=zz1w7C+ZtPl70aI2eAmZ/T8nTLh8rqEFYYRBP4u2ho4=; h=Date:To:Subject:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=dSfJkD+H5fYPlWO8J0UD6wdMpwJiuIfmV69jxCl6MzVLwii5z6mKkOfeKCz6nhvxJ ZZ9enj3SBeRx53FO0HPnMw624qPFuR21HwScbFZW/B7SPDCTTr7tIaS2ZPdaLKPM6p j+P9Das6S5+mi8DJXJNp0FtLJ6tbACIgco5q/dew= Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by sourceware.org (Postfix) with ESMTPS id AB49E3857828 for ; Thu, 5 Aug 2021 08:44:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AB49E3857828 Received: by mail.kernel.org (Postfix) with ESMTPSA id 7E03B610CC; Thu, 5 Aug 2021 08:44:12 +0000 (UTC) Received: by pali.im (Postfix) id 5EEEE817; Thu, 5 Aug 2021 10:44:10 +0200 (CEST) Date: Thu, 5 Aug 2021 10:44:10 +0200 To: Greg Kroah-Hartman Subject: Re: [PATCH v3] ioctl_tty.2: Add example how to get or set baudrate on the serial port Message-ID: <20210805084410.sb5lybdri6r7t2da@pali> References: <20210730095333.6118-1-pali@kernel.org> <20210801135146.14849-1-pali@kernel.org> <20210804220808.cetleob6dldpfnjk@pali> <20210805082243.qciylqnt5g74if7i@pali> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20180716 X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: =?utf-8?q?Pali_Roh=C3=A1r_via_Libc-alpha?= Reply-To: Pali =?utf-8?B?Um9ow6Fy?= Cc: Marek =?utf-8?B?QmVow7pu?= , Alejandro Colomar , libc-alpha@sourceware.org, linux-man@vger.kernel.org, Michael Kerrisk , linux-serial@vger.kernel.org, "G. Branden Robinson" Errors-To: libc-alpha-bounces+e=80x24.org@sourceware.org Sender: "Libc-alpha" On Thursday 05 August 2021 10:30:15 Greg Kroah-Hartman wrote: > 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 > > > > > > > > > > --- > > > > > 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 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 > > > > > +#include > > > > > +#include > > > > > +#include > > > > > +#include > > > > > +#include > > > > > +#include > > > > > + > > > > > +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. Exactly, and this is one of the reason why I sent this my patch for ioctl_tty.2. > Here's one that I point people to a lot: > https://github.com/GrantEdwards/Linux-arbitrary-baud I'm looking at this example at it has lot of problems: * Does not compile on powerpc (see explanation above). * Does not include and instead provide open-coded declaration of ioctl: int ioctl(int d, int request, ...); * Does not handle case when TCGETS/TCSETS contains t.c_ospeed In my opinion include header files should be used instead of writing own declaration of functions. > 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... Hm... I do not see any license in other manpage examples. Does not apply for it global license defined in ioctl_tty.2 file? Alejandro: How do you handle licences in other examples? > thanks, > > greg k-h