Re: [PATCH 1/3] termbits.h: create termbits-common.h for identical bits
On Mon, 9 May 2022, Helge Deller wrote:
> Hello Ilpo,
>
> On 5/9/22 11:34, Ilpo Järvinen wrote:
> > Some defines are the same across all archs. Move the most obvious
> > intersection to termbits-common.h.
>
> I like your cleanup patches, but in this specific one, does it makes sense
> to split up together-belonging constants, e.g.
>
> > diff --git a/arch/parisc/include/uapi/asm/termbits.h
> > b/arch/parisc/include/uapi/asm/termbits.h
> > index 6017ee08f099..7f74a822b7ea 100644
> > --- a/arch/parisc/include/uapi/asm/termbits.h
> > +++ b/arch/parisc/include/uapi/asm/termbits.h
> > @@ -61,31 +61,15 @@ struct ktermios {
> >
> >
> > /* c_iflag bits */
> > -#define IGNBRK 0x1
> > -#define BRKINT 0x2
> > -#define IGNPAR 0x4
> > -#define PARMRK 0x8
> > -#define INPCK 0x00010
> > -#define ISTRIP 0x00020
> > -#define INLCR 0x00040
> > -#define IGNCR 0x00080
> > -#define ICRNL 0x00100
> > #define IUCLC 0x00200
> > #define IXON 0x00400
> > -#define IXANY 0x00800
> > #define IXOFF 0x01000
> > #define IMAXBEL0x04000
> > #define IUTF8 0x08000
>
> In the hunk above you leave IUCLC, IXON, IXOFF... because they seem unique to
> parisc.
> The other defines are then taken from generic header.
> Although this is correct, it leaves single values alone, which make it hard
> to verify
> because you don't see the full list of values in one place.
While I too am fine either way, I don't think these are as strongly
grouped as you seem to imply. There's no big advantage in having as much
as possible within the same file. If somebody is looking for the meaning
of these, these headers are no match when compared e.g. with stty manpage.
For c_iflag, the break, parity and cr related "groups" within c_iflag are
moving completely to common header.
IXANY is probably only one close to borderline whether it kind of belongs
to the same group as IXON/IXOFF (which both by chance both remained on the
same side in the split). I don't think it does strongly enough to warrant
keeping them next to each other but I'm open what opinions others have on
it.
The rest in c_iflag don't seem to be strongly tied/grouped to the other
defines within c_iflag. They're just bits that appear next/close to each
other but are not tied by any significant meaning-based connection.
C_oflag is more messy. I exercised grouping based judgement with c_oflag
where only the defines with all bits as zero would have moved to the
common header breaking the groups very badly. That is, only CR0 would have
moved and CR1-3 remained in arch headers, etc. which made no sense to do.
One could argue, that since ONLCR (and perhaps CRDLY) are not moving, no
other cr related defines should move either.
> > @@ -112,24 +96,6 @@ struct ktermios {
> >
> > /* c_cflag bit meaning */
> > #define CBAUD 0x100f
> > -#define B00x /* hang up */
> > -#define B50 0x0001
> > -#define B75 0x0002
> > -#define B110 0x0003
> > -#define B134 0x0004
> > -#define B150 0x0005
> > -#define B200 0x0006
> > -#define B300 0x0007
> > -#define B600 0x0008
> > -#define B1200 0x0009
> > -#define B1800 0x000a
> > -#define B2400 0x000b
> > -#define B4800 0x000c
> > -#define B9600 0x000d
> > -#define B192000x000e
> > -#define B384000x000f
> > -#define EXTA B19200
> > -#define EXTB B38400
>
> Here all baud values are dropped and will be taken from generic header,
> which is good.
>
> That said, I think it's good to move away the second hunk,
> but maybe we should keep the first as is?
>
> It's just a thought. Either way, I'm fine your patch if that's the
> way which is decided to go for all platforms.
Yes, lets wait and see what the others think.
Thanks for taking a look!
--
i.
Re: [PATCH 1/3] termbits.h: create termbits-common.h for identical bits
Hello Ilpo,
On 5/9/22 11:34, Ilpo Järvinen wrote:
> Some defines are the same across all archs. Move the most obvious
> intersection to termbits-common.h.
I like your cleanup patches, but in this specific one, does it makes sense
to split up together-belonging constants, e.g.
> diff --git a/arch/parisc/include/uapi/asm/termbits.h
> b/arch/parisc/include/uapi/asm/termbits.h
> index 6017ee08f099..7f74a822b7ea 100644
> --- a/arch/parisc/include/uapi/asm/termbits.h
> +++ b/arch/parisc/include/uapi/asm/termbits.h
> @@ -61,31 +61,15 @@ struct ktermios {
>
>
> /* c_iflag bits */
> -#define IGNBRK 0x1
> -#define BRKINT 0x2
> -#define IGNPAR 0x4
> -#define PARMRK 0x8
> -#define INPCK0x00010
> -#define ISTRIP 0x00020
> -#define INLCR0x00040
> -#define IGNCR0x00080
> -#define ICRNL0x00100
> #define IUCLC0x00200
> #define IXON 0x00400
> -#define IXANY0x00800
> #define IXOFF0x01000
> #define IMAXBEL 0x04000
> #define IUTF80x08000
In the hunk above you leave IUCLC, IXON, IXOFF... because they seem unique to
parisc.
The other defines are then taken from generic header.
Although this is correct, it leaves single values alone, which make it hard to
verify
because you don't see the full list of values in one place.
> @@ -112,24 +96,6 @@ struct ktermios {
>
> /* c_cflag bit meaning */
> #define CBAUD0x100f
> -#define B0 0x /* hang up */
> -#define B50 0x0001
> -#define B75 0x0002
> -#define B1100x0003
> -#define B1340x0004
> -#define B1500x0005
> -#define B2000x0006
> -#define B3000x0007
> -#define B6000x0008
> -#define B1200 0x0009
> -#define B1800 0x000a
> -#define B2400 0x000b
> -#define B4800 0x000c
> -#define B9600 0x000d
> -#define B19200 0x000e
> -#define B38400 0x000f
> -#define EXTA B19200
> -#define EXTB B38400
Here all baud values are dropped and will be taken from generic header, which
is good.
That said, I think it's good to move away the second hunk,
but maybe we should keep the first as is?
It's just a thought. Either way, I'm fine your patch if that's the
way which is decided to go for all platforms.
Helge
[PATCH 1/3] termbits.h: create termbits-common.h for identical bits
Some defines are the same across all archs. Move the most obvious
intersection to termbits-common.h.
Signed-off-by: Ilpo Järvinen
---
arch/alpha/include/uapi/asm/termbits.h | 52 +
arch/mips/include/uapi/asm/termbits.h | 53 +-
arch/parisc/include/uapi/asm/termbits.h| 54 +-
arch/powerpc/include/uapi/asm/termbits.h | 52 +
arch/sparc/include/uapi/asm/termbits.h | 53 +-
include/uapi/asm-generic/termbits-common.h | 65 ++
include/uapi/asm-generic/termbits.h| 53 +-
7 files changed, 76 insertions(+), 306 deletions(-)
create mode 100644 include/uapi/asm-generic/termbits-common.h
diff --git a/arch/alpha/include/uapi/asm/termbits.h
b/arch/alpha/include/uapi/asm/termbits.h
index 30dc7ff777b8..3c7a9afc4333 100644
--- a/arch/alpha/include/uapi/asm/termbits.h
+++ b/arch/alpha/include/uapi/asm/termbits.h
@@ -4,8 +4,8 @@
#include
-typedef unsigned char cc_t;
-typedef unsigned int speed_t;
+#include
+
typedef unsigned int tcflag_t;
/*
@@ -72,33 +72,17 @@ struct ktermios {
#define VTIME 17
/* c_iflag bits */
-#define IGNBRK 0x1
-#define BRKINT 0x2
-#define IGNPAR 0x4
-#define PARMRK 0x8
-#define INPCK 0x00010
-#define ISTRIP 0x00020
-#define INLCR 0x00040
-#define IGNCR 0x00080
-#define ICRNL 0x00100
#define IXON 0x00200
#define IXOFF 0x00400
-#define IXANY 0x00800
#define IUCLC 0x01000
#define IMAXBEL0x02000
#define IUTF8 0x04000
/* c_oflag bits */
-#define OPOST 0x1
#define ONLCR 0x2
#define OLCUC 0x4
-#define OCRNL 0x8
-#define ONOCR 0x00010
-#define ONLRET 0x00020
-#define OFILL 0x40
-#define OFDEL 0x80
#define NLDLY 0x000300
#define NL0 0x00
#define NL1 0x000100
@@ -131,24 +115,6 @@ struct ktermios {
/* c_cflag bit meaning */
#define CBAUD 0x001f
-#define B00x /* hang up */
-#define B50 0x0001
-#define B75 0x0002
-#define B110 0x0003
-#define B134 0x0004
-#define B150 0x0005
-#define B200 0x0006
-#define B300 0x0007
-#define B600 0x0008
-#define B1200 0x0009
-#define B1800 0x000a
-#define B2400 0x000b
-#define B4800 0x000c
-#define B9600 0x000d
-#define B192000x000e
-#define B384000x000f
-#define EXTA B19200
-#define EXTB B38400
#define CBAUDEX0x
#define B576000x0010
#define B115200 0x0011
@@ -180,11 +146,8 @@ struct ktermios {
#define HUPCL 0x4000
#define CLOCAL 0x8000
-#define CMSPAR 0x4000 /* mark or space (stick) parity */
-#define CRTSCTS0x8000 /* flow control */
#define CIBAUD 0x1f
-#define IBSHIFT16
/* c_lflag bits */
#define ISIG 0x0080
@@ -204,17 +167,6 @@ struct ktermios {
#define IEXTEN 0x0400
#define EXTPROC0x1000
-/* Values for the ACTION argument to `tcflow'. */
-#defineTCOOFF 0
-#defineTCOON 1
-#defineTCIOFF 2
-#defineTCION 3
-
-/* Values for the QUEUE_SELECTOR argument to `tcflush'. */
-#defineTCIFLUSH0
-#defineTCOFLUSH1
-#defineTCIOFLUSH 2
-
/* Values for the OPTIONAL_ACTIONS argument to `tcsetattr'. */
#defineTCSANOW 0
#defineTCSADRAIN 1
diff --git a/arch/mips/include/uapi/asm/termbits.h
b/arch/mips/include/uapi/asm/termbits.h
index d1315ccdb39a..b33f514315ce 100644
--- a/arch/mips/include/uapi/asm/termbits.h
+++ b/arch/mips/include/uapi/asm/termbits.h
@@ -13,8 +13,8 @@
#include
-typedef unsigned char cc_t;
-typedef unsigned int speed_t;
+#include
+
typedef unsigned int tcflag_t;
/*
@@ -80,31 +80,15 @@ struct ktermios {
#define VEOL 17 /* End-of-line character [ICANON]. */
/* c_iflag bits */
-#define IGNBRK 0x1 /* Ignore break condition. */
-#define BRKINT 0x2 /* Signal interrupt on break. */
-#define IGNPAR 0x4 /* Ignore characters with parity errors. */
-#define PARMRK 0x8 /* Mark parity and framing errors. */
-#define INPCK 0x00010 /* Enable input parity check. */
-#define ISTRIP 0x00020 /* Strip 8th bit off characters. */
-#define INLCR 0x00040 /* Map NL to CR on input. */
-#define IGNCR 0x00080 /* Ignore CR. */
-#define ICRNL 0x00100 /* Map CR to NL on input. */
#define IUCLC 0x00200 /* Map upper case to lower case on input. */
#define IXON 0x00400 /* Enable start/stop output control. */
-#define IXANY 0x00800 /* Any character will restart after stop. */
#define IXOFF
