On Mon, Dec 11, 2023 at 03:07:59PM +0200, 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. How about revising the name of 'TIOCSRS485' and 'serial_rs485' to a general one, and put RS422 and RS485 configuration flags into that structure? So that in userspace it could set RS422 or RS485 configurations using a single ioctl command and one structure. In this way, it won't be confused in userspace and won't add new data structure internally as well. --- Sincerely, Crescent Hsieh.
