1. Patchew
  2. linux
  3. [PATCH v6 0/2] tty: serial: Add RS422 serial interface
  4. View patch

[PATCH v6 1/2] tty: serial: Cleanup the bit shift with macro

Crescent CY Hsieh posted 2 patches 2 years ago
[PATCH v6 1/2] tty: serial: Cleanup the bit shift with macro
Posted by Crescent CY Hsieh 2 years ago 
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)
-#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;
-- 
2.34.1
Re: [PATCH v6 1/2] tty: serial: Cleanup the bit shift with macro
Posted by Christoph Niedermaier 1 year, 11 months ago 
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
Re: [PATCH v6 1/2] tty: serial: Cleanup the bit shift with macro
Posted by Greg KH 1 year, 11 months ago 
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
RE: [PATCH v6 1/2] tty: serial: Cleanup the bit shift with macro
Posted by Christoph Niedermaier 1 year, 11 months ago 
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
Re: [PATCH v6 1/2] tty: serial: Cleanup the bit shift with macro
Posted by Crescent CY Hsieh 1 year, 11 months ago 
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
Re: [PATCH v6 1/2] tty: serial: Cleanup the bit shift with macro
Posted by Greg KH 1 year, 11 months ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.