On Sat, 20 Aug 2022, Maciej W. Rozycki wrote: > On Thu, 18 Aug 2022, Ilpo Järvinen wrote: > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> > > > for all, but patch 3. > > > > > > I'm not sure we can blindly use old_termios settings, is there any > > > guarantee that old_termios _always_ has a correct baud rate settings? > > > > Old_termios is just the previous termios the port was using. If the > > baudrate in termios was invalid already by then, it's another issue that > > should be fixed (but I cannot see how that could occur assuming the > > validation works). > > > > How could it get wrong baud rate settings if the kernel sets (old_)termios > > (earlier) through these same paths (which validated it)? > > If the old baud rate in termios was invalid, then we would just resort to > 9600 baud, so I wouldn't be concerned here as we need to set the baud rate > to something anyway. I still assert that old baud rate should never be "invalid". When it was set (earlier) through dz_set_termios(), it got validated and invalid ones were corrected. I think a valid baud rate in tty->termios is an invariant we just maintain on each call to dz_set_termios(). > My only concern is whether `tty_termios_baud_rate' > can ever return 0 for `old_termios', which would of course never happen > with `uart_get_baud_rate'. dz_encode_baud_rate() disallows 0 as baud rate. I don't understand what you're trying to say here. > But new code seems to me to be doing the right thing anyway. That is if > we're getting out of a hangup (which we don't currently handle with the > modem lines anyway, but that's quite a different and complex matter) with > an invalid baud rate, then we'll fail to encode it, then fail to encode 0, > and finally resort to 9600 baud and write it back to `termios'. So we'll > get out of a hangup with a baud rate different to one requested, but that > is as much as we can do in that case: we have fulfilled the request the > best we could and `uart_get_baud_rate' would set the rate to 9600 baud > anyway. This I kind of agree with but given the precondition where 0 baud rate is always rejected by dz_encode_baud_rate(), I don't think this scenario can materialize at all? > Given the observation above I have acked your patch. Perhaps you could > put some of the analysis above into the change description. > > Maciej > -- i.
