Hi, On 11.12.23 14:07, Andy Shevchenko wrote: > On Sat, Dec 09, 2023 at 12:47:47PM +0100, Lino Sanfilippo wrote: >> On 06.12.23 16:42, Lino Sanfilippo wrote: > >>>>>> Crescent CY Hsieh (+cc) is in parallel trying to add an RS-422 mode bit >>>>>> to struct serial_rs485: >>>>>> >>>>>> https://lore.kernel.org/all/20231121095122.15948-1-crescentcy.hsieh@xxxxxxxx/ >>>>>> >>>>> >>>>> That new flag was suggested by me instead of using SER_RS422_ENABLED, which >>>>> would mostly be redundant to SER_RS485_ENABLED. >> >> A cleaner solution would probably be to not handle RS422 with the RS485 settings at >> all, but to introduce another set of ioctls to set and read it. >> >> An own RS422 structure like >> >> struct serial_rs422 { >> __u32 flags; >> #define SER_RS422_ENABLED (1 << 0) >> #define SER_RS422_TERMINATE_BUS (1 << 1) >> }; >> >> >> could be used as the parameter for these new ioctls. >> >> Any comments on this? > > I have (maybe not so constructive) a comment. Please, at all means try to not > extend the existing serial data structures, we have too many ones with too many > fields already. For user space, though, one may use unions and flags, but for > internal ones it might be better ways, I think. > Ok, thanks. This is still a valuable information. So what if the above structure (serial_rs422) is only used as a parameter of a new TIOCSRS422 ioctl and only internally we set a SER_RS485_MODE_RS422 flag in the serial_rs485 struct? So we do not have to add something new to uart_port but also do not expose the mixture of RS485 and RS422 settings within the serial_rs485 structure to userspace. Regards, Lino
