* Re: [PATCH v6 1/2] tty: serial: Cleanup the bit shift with macro
@ 2024-01-16 15:58 Christoph Niedermaier
2024-01-17 14:56 ` Christoph Niedermaier
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Niedermaier @ 2024-01-16 15:58 UTC (permalink / raw)
To: crescentcy.hsieh
Cc: gregkh, jirislaby, linux-kernel, linux-serial, cniedermaier,
linux-arm-kernel
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6 1/2] tty: serial: Cleanup the bit shift with macro
2024-01-16 15:58 [PATCH v6 1/2] tty: serial: Cleanup the bit shift with macro Christoph Niedermaier
@ 2024-01-17 14:56 ` Christoph Niedermaier
2024-01-18 7:01 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Niedermaier @ 2024-01-17 14:56 UTC (permalink / raw)
To: crescentcy.hsieh
Cc: gregkh, jirislaby, LinoSanfilippo, lukas, linux-kernel,
linux-serial, cniedermaier, linux-arm-kernel
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6 1/2] tty: serial: Cleanup the bit shift with macro
2024-01-17 14:56 ` Christoph Niedermaier
@ 2024-01-18 7:01 ` Greg KH
2024-01-18 8:14 ` Crescent CY Hsieh
2024-01-18 9:29 ` Christoph Niedermaier
0 siblings, 2 replies; 6+ messages in thread
From: Greg KH @ 2024-01-18 7:01 UTC (permalink / raw)
To: Christoph Niedermaier
Cc: crescentcy.hsieh, jirislaby, LinoSanfilippo, lukas, linux-kernel,
linux-serial, linux-arm-kernel
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6 1/2] tty: serial: Cleanup the bit shift with macro
2024-01-18 7:01 ` Greg KH
@ 2024-01-18 8:14 ` Crescent CY Hsieh
2024-01-18 8:44 ` Greg KH
2024-01-18 9:29 ` Christoph Niedermaier
1 sibling, 1 reply; 6+ messages in thread
From: Crescent CY Hsieh @ 2024-01-18 8:14 UTC (permalink / raw)
To: Greg KH
Cc: Christoph Niedermaier, jirislaby, LinoSanfilippo, lukas,
linux-kernel, linux-serial, linux-arm-kernel
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6 1/2] tty: serial: Cleanup the bit shift with macro
2024-01-18 8:14 ` Crescent CY Hsieh
@ 2024-01-18 8:44 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2024-01-18 8:44 UTC (permalink / raw)
To: Crescent CY Hsieh
Cc: Christoph Niedermaier, jirislaby, LinoSanfilippo, lukas,
linux-kernel, linux-serial, linux-arm-kernel
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v6 1/2] tty: serial: Cleanup the bit shift with macro
2024-01-18 7:01 ` Greg KH
2024-01-18 8:14 ` Crescent CY Hsieh
@ 2024-01-18 9:29 ` Christoph Niedermaier
1 sibling, 0 replies; 6+ messages in thread
From: Christoph Niedermaier @ 2024-01-18 9:29 UTC (permalink / raw)
To: Greg KH
Cc: crescentcy.hsieh@moxa.com, jirislaby@kernel.org,
LinoSanfilippo@gmx.de, lukas@wunner.de,
linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-01-18 9:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-16 15:58 [PATCH v6 1/2] tty: serial: Cleanup the bit shift with macro Christoph Niedermaier
2024-01-17 14:56 ` Christoph Niedermaier
2024-01-18 7:01 ` Greg KH
2024-01-18 8:14 ` Crescent CY Hsieh
2024-01-18 8:44 ` Greg KH
2024-01-18 9:29 ` Christoph Niedermaier
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 NNTP newsgroup(s).