This patch series adds RS422 flag to switch the serial interface into
RS422 if supported by using "TIOCSRS485" ioctl command.
---
Changes from v5 to v6:
- Split patch into cleanup patch and new feature patch
- [PATCH v6 2/2]:
- Add termination resistors support for RS422
Changes from v4 to v5:
- Revise commit message.
- Delete RS422 checks within uart_set_rs485_termination().
Changes from v3 to v4:
- Include 'linux/const.h' header in '/include/uapi/linux/serial.h'
- Replace BIT() with _BITUL() which defined in
'/include/uapi/linux/const.h'
Changes from v2 to v3:
- Remove "SER_RS422_ENABLED" flag from legacy flags.
- Revise "SER_RS422_ENABLED" into "SER_RS485_MODE_RS422".
- Remove the code which checks the conflicts between SER_RS485_ENABLED
and SER_RS422_ENABLED.
- Add return check in uart_set_rs485_termination().
Changes from v1 to v2:
- Revise the logic that checks whether RS422/RS485 are enabled
simultaneously.
v5: https://lore.kernel.org/all/[email protected]/
v4: https://lore.kernel.org/all/[email protected]/
v3: https://lore.kernel.org/all/[email protected]/
v2: https://lore.kernel.org/all/[email protected]/
v1: https://lore.kernel.org/all/[email protected]/
Crescent CY Hsieh (2):
tty: serial: Cleanup the bit shift with macro
tty: serial: Add RS422 flag to struct serial_rs485
drivers/tty/serial/serial_core.c | 6 ++++++
include/uapi/linux/serial.h | 19 +++++++++++--------
2 files changed, 17 insertions(+), 8 deletions(-)
--
2.34.1
This patch replaces the bit shift code with "_BITUL()" macro inside
"serial_rs485" struct.
Signed-off-by: Crescent CY Hsieh <[email protected]>
---
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
Add "SER_RS485_MODE_RS422" flag to struct serial_rs485, so that serial
port can switch interface into RS422 if supported by using ioctl command
"TIOCSRS485".
By treating RS422 as a mode of RS485, which means while enabling RS422
there are two flags need to be set (SER_RS485_ENABLED and
SER_RS485_MODE_RS422), it would make things much easier. For example
some places that checks for "SER_RS485_ENABLED" won't need to be rewritten.
Signed-off-by: Crescent CY Hsieh <[email protected]>
---
drivers/tty/serial/serial_core.c | 6 ++++++
include/uapi/linux/serial.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 831d03361..a195ec1ab 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1376,6 +1376,12 @@ static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs4
return;
}
+ /* Clear other RS485 flags but SER_RS485_TERMINATE_BUS and return if enabling RS422 */
+ if (rs485->flags & SER_RS485_MODE_RS422) {
+ rs485->flags &= (SER_RS485_ENABLED | SER_RS485_MODE_RS422 | SER_RS485_TERMINATE_BUS);
+ return;
+ }
+
/* Pick sane settings if the user hasn't */
if ((supported_flags & (SER_RS485_RTS_ON_SEND|SER_RS485_RTS_AFTER_SEND)) &&
!(rs485->flags & SER_RS485_RTS_ON_SEND) ==
diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
index 6c75ebdd7..9086367db 100644
--- a/include/uapi/linux/serial.h
+++ b/include/uapi/linux/serial.h
@@ -138,6 +138,7 @@ struct serial_icounter_struct {
* * %SER_RS485_ADDRB - Enable RS485 addressing mode.
* * %SER_RS485_ADDR_RECV - Receive address filter (enables @addr_recv). Requires %SER_RS485_ADDRB.
* * %SER_RS485_ADDR_DEST - Destination address (enables @addr_dest). Requires %SER_RS485_ADDRB.
+ * * %SER_RS485_MODE_RS422 - Enable RS422. Requires %SER_RS485_ENABLED.
*/
struct serial_rs485 {
__u32 flags;
@@ -149,6 +150,7 @@ struct serial_rs485 {
#define SER_RS485_ADDRB _BITUL(5)
#define SER_RS485_ADDR_RECV _BITUL(6)
#define SER_RS485_ADDR_DEST _BITUL(7)
+#define SER_RS485_MODE_RS422 _BITUL(8)
__u32 delay_rts_before_send;
__u32 delay_rts_after_send;
--
2.34.1
Hi everyone,
> This patch replaces the bit shift code with "_BITUL()" macro inside
> "serial_rs485" struct.
>
> Signed-off-by: Crescent CY Hsieh <[email protected]>
> ---
> 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 <[email protected]>
> > ---
> > 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 <[email protected]>
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 <[email protected]>
>>> ---
>>> 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