| Message ID | 20231201071554.258607-2-crescentcy.hsieh@moxa.com |
|---|---|
| State | New |
| Headers | show |
| Series | tty: serial: Add RS422 serial interface | expand |
Hi everyone, > This patch replaces the bit shift code with "_BITUL()" macro inside > "serial_rs485" struct. > > Signed-off-by: Crescent CY Hsieh <crescentcy.hsieh@moxa.com> > --- > include/uapi/linux/serial.h | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h > index 53bc1af67..6c75ebdd7 100644 > --- a/include/uapi/linux/serial.h > +++ b/include/uapi/linux/serial.h > @@ -11,6 +11,7 @@ > #ifndef _UAPI_LINUX_SERIAL_H > #define _UAPI_LINUX_SERIAL_H > > +#include <linux/const.h> > #include <linux/types.h> > > #include <linux/tty_flags.h> > @@ -140,14 +141,14 @@ struct serial_icounter_struct { > */ > struct serial_rs485 { > __u32 flags; > -#define SER_RS485_ENABLED (1 << 0) > -#define SER_RS485_RTS_ON_SEND (1 << 1) > -#define SER_RS485_RTS_AFTER_SEND (1 << 2) In the old definition (1 << 3) wasn't used. > -#define SER_RS485_RX_DURING_TX (1 << 4) > -#define SER_RS485_TERMINATE_BUS (1 << 5) > -#define SER_RS485_ADDRB (1 << 6) > -#define SER_RS485_ADDR_RECV (1 << 7) > -#define SER_RS485_ADDR_DEST (1 << 8) > +#define SER_RS485_ENABLED _BITUL(0) > +#define SER_RS485_RTS_ON_SEND _BITUL(1) > +#define SER_RS485_RTS_AFTER_SEND _BITUL(2) > +#define SER_RS485_RX_DURING_TX _BITUL(3) Isn't it a break if number 3 isn't skipped here as well? Regards Christoph
On Wed, Jan 17, 2024 at 03:56:23PM +0100, Christoph Niedermaier wrote: > Hi everyone, > > > This patch replaces the bit shift code with "_BITUL()" macro inside > > "serial_rs485" struct. > > > > Signed-off-by: Crescent CY Hsieh <crescentcy.hsieh@moxa.com> > > --- > > include/uapi/linux/serial.h | 17 +++++++++-------- > > 1 file changed, 9 insertions(+), 8 deletions(-) > > > > diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h > > index 53bc1af67..6c75ebdd7 100644 > > --- a/include/uapi/linux/serial.h > > +++ b/include/uapi/linux/serial.h > > @@ -11,6 +11,7 @@ > > #ifndef _UAPI_LINUX_SERIAL_H > > #define _UAPI_LINUX_SERIAL_H > > > > +#include <linux/const.h> > > #include <linux/types.h> > > > > #include <linux/tty_flags.h> > > @@ -140,14 +141,14 @@ struct serial_icounter_struct { > > */ > > struct serial_rs485 { > > __u32 flags; > > -#define SER_RS485_ENABLED (1 << 0) > > -#define SER_RS485_RTS_ON_SEND (1 << 1) > > -#define SER_RS485_RTS_AFTER_SEND (1 << 2) > > In the old definition (1 << 3) wasn't used. > > > -#define SER_RS485_RX_DURING_TX (1 << 4) > > -#define SER_RS485_TERMINATE_BUS (1 << 5) > > -#define SER_RS485_ADDRB (1 << 6) > > -#define SER_RS485_ADDR_RECV (1 << 7) > > -#define SER_RS485_ADDR_DEST (1 << 8) > > +#define SER_RS485_ENABLED _BITUL(0) > > +#define SER_RS485_RTS_ON_SEND _BITUL(1) > > +#define SER_RS485_RTS_AFTER_SEND _BITUL(2) > > +#define SER_RS485_RX_DURING_TX _BITUL(3) > > Isn't it a break if number 3 isn't skipped here as well? Ugh, yes it is, good catch! Care to send a patch to fix this up? thanks, greg k-h
On Thu, Jan 18, 2024 at 08:01:58AM +0100, Greg KH wrote: > On Wed, Jan 17, 2024 at 03:56:23PM +0100, Christoph Niedermaier wrote: > > > struct serial_rs485 { > > > __u32 flags; > > > -#define SER_RS485_ENABLED (1 << 0) > > > -#define SER_RS485_RTS_ON_SEND (1 << 1) > > > -#define SER_RS485_RTS_AFTER_SEND (1 << 2) > > > > In the old definition (1 << 3) wasn't used. > > > > > -#define SER_RS485_RX_DURING_TX (1 << 4) > > > -#define SER_RS485_TERMINATE_BUS (1 << 5) > > > -#define SER_RS485_ADDRB (1 << 6) > > > -#define SER_RS485_ADDR_RECV (1 << 7) > > > -#define SER_RS485_ADDR_DEST (1 << 8) > > > +#define SER_RS485_ENABLED _BITUL(0) > > > +#define SER_RS485_RTS_ON_SEND _BITUL(1) > > > +#define SER_RS485_RTS_AFTER_SEND _BITUL(2) > > > +#define SER_RS485_RX_DURING_TX _BITUL(3) > > > > Isn't it a break if number 3 isn't skipped here as well? Sorry I might have misunderstood the meaning of "broke userspace". In this case, does it imply splitting the "cleanup" patch and the "add feature" patch, or leaving the third bit unused? Or perhaps both? --- Sincerely, Crescent Hsieh
On Thu, Jan 18, 2024 at 04:14:41PM +0800, Crescent CY Hsieh wrote: > On Thu, Jan 18, 2024 at 08:01:58AM +0100, Greg KH wrote: > > On Wed, Jan 17, 2024 at 03:56:23PM +0100, Christoph Niedermaier wrote: > > > > struct serial_rs485 { > > > > __u32 flags; > > > > -#define SER_RS485_ENABLED (1 << 0) > > > > -#define SER_RS485_RTS_ON_SEND (1 << 1) > > > > -#define SER_RS485_RTS_AFTER_SEND (1 << 2) > > > > > > In the old definition (1 << 3) wasn't used. > > > > > > > -#define SER_RS485_RX_DURING_TX (1 << 4) > > > > -#define SER_RS485_TERMINATE_BUS (1 << 5) > > > > -#define SER_RS485_ADDRB (1 << 6) > > > > -#define SER_RS485_ADDR_RECV (1 << 7) > > > > -#define SER_RS485_ADDR_DEST (1 << 8) > > > > +#define SER_RS485_ENABLED _BITUL(0) > > > > +#define SER_RS485_RTS_ON_SEND _BITUL(1) > > > > +#define SER_RS485_RTS_AFTER_SEND _BITUL(2) > > > > +#define SER_RS485_RX_DURING_TX _BITUL(3) > > > > > > Isn't it a break if number 3 isn't skipped here as well? > > Sorry I might have misunderstood the meaning of "broke userspace". > > In this case, does it imply splitting the "cleanup" patch and the "add > feature" patch, or leaving the third bit unused? Or perhaps both? You can not redefine existing defines to different values, like you did here, that changes the user/kernel api of the system. Luckily this isn't in any released kernel, I'll either revert this after -rc1 is out, or take a patch from anyone else to do so if they want to send it now. thanks, greg k-h
From: Greg KH <gregkh@linuxfoundation.org> Sent: Thursday, January 18, 2024 8:02 AM > On Wed, Jan 17, 2024 at 03:56:23PM +0100, Christoph Niedermaier wrote: >> Hi everyone, >> >>> This patch replaces the bit shift code with "_BITUL()" macro inside >>> "serial_rs485" struct. >>> >>> Signed-off-by: Crescent CY Hsieh <crescentcy.hsieh@moxa.com> >>> --- >>> include/uapi/linux/serial.h | 17 +++++++++-------- >>> 1 file changed, 9 insertions(+), 8 deletions(-) >>> >>> diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h >>> index 53bc1af67..6c75ebdd7 100644 >>> --- a/include/uapi/linux/serial.h >>> +++ b/include/uapi/linux/serial.h >>> @@ -11,6 +11,7 @@ >>> #ifndef _UAPI_LINUX_SERIAL_H >>> #define _UAPI_LINUX_SERIAL_H >>> >>> +#include <linux/const.h> >>> #include <linux/types.h> >>> >>> #include <linux/tty_flags.h> >>> @@ -140,14 +141,14 @@ struct serial_icounter_struct { >>> */ >>> struct serial_rs485 { >>> __u32 flags; >>> -#define SER_RS485_ENABLED (1 << 0) >>> -#define SER_RS485_RTS_ON_SEND (1 << 1) >>> -#define SER_RS485_RTS_AFTER_SEND (1 << 2) >> >> In the old definition (1 << 3) wasn't used. >> >>> -#define SER_RS485_RX_DURING_TX (1 << 4) >>> -#define SER_RS485_TERMINATE_BUS (1 << 5) >>> -#define SER_RS485_ADDRB (1 << 6) >>> -#define SER_RS485_ADDR_RECV (1 << 7) >>> -#define SER_RS485_ADDR_DEST (1 << 8) >>> +#define SER_RS485_ENABLED _BITUL(0) >>> +#define SER_RS485_RTS_ON_SEND _BITUL(1) >>> +#define SER_RS485_RTS_AFTER_SEND _BITUL(2) >>> +#define SER_RS485_RX_DURING_TX _BITUL(3) >> >> Isn't it a break if number 3 isn't skipped here as well? > > Ugh, yes it is, good catch! > > Care to send a patch to fix this up? OK, I'll make a patch. Regards Christoph
diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h index 53bc1af67..6c75ebdd7 100644 --- a/include/uapi/linux/serial.h +++ b/include/uapi/linux/serial.h @@ -11,6 +11,7 @@ #ifndef _UAPI_LINUX_SERIAL_H #define _UAPI_LINUX_SERIAL_H +#include <linux/const.h> #include <linux/types.h> #include <linux/tty_flags.h> @@ -140,14 +141,14 @@ struct serial_icounter_struct { */ struct serial_rs485 { __u32 flags; -#define SER_RS485_ENABLED (1 << 0) -#define SER_RS485_RTS_ON_SEND (1 << 1) -#define SER_RS485_RTS_AFTER_SEND (1 << 2) -#define SER_RS485_RX_DURING_TX (1 << 4) -#define SER_RS485_TERMINATE_BUS (1 << 5) -#define SER_RS485_ADDRB (1 << 6) -#define SER_RS485_ADDR_RECV (1 << 7) -#define SER_RS485_ADDR_DEST (1 << 8) +#define SER_RS485_ENABLED _BITUL(0) +#define SER_RS485_RTS_ON_SEND _BITUL(1) +#define SER_RS485_RTS_AFTER_SEND _BITUL(2) +#define SER_RS485_RX_DURING_TX _BITUL(3) +#define SER_RS485_TERMINATE_BUS _BITUL(4) +#define SER_RS485_ADDRB _BITUL(5) +#define SER_RS485_ADDR_RECV _BITUL(6) +#define SER_RS485_ADDR_DEST _BITUL(7) __u32 delay_rts_before_send; __u32 delay_rts_after_send;
This patch replaces the bit shift code with "_BITUL()" macro inside "serial_rs485" struct. Signed-off-by: Crescent CY Hsieh <crescentcy.hsieh@moxa.com> --- include/uapi/linux/serial.h | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)