On Mon, 18 Apr 2022, Andy Shevchenko wrote: > > With the baud base set to 15625000 and the unsigned 16-bit UART_DIV_MAX > > limitation imposed by `serial8250_get_baud_rate' standard baud rates > > below 300bps become unavailable in the regular way, e.g. the rate of > > 200bps requires the baud base to be divided by 78125 and that is beyond > > the unsigned 16-bit range. The historic spd_cust feature can still be > > used to obtain such rates if so required. > > > > See Documentation/tty/device_drivers/oxsemi-tornado.rst for more details. > > I'm not sure I understand how this change can have the 8250_port > changes which were done in the previous patches. What did I miss? You mean the `->mcr' part? It's required to keep the CLKSEL bit set at all times. > Also, looking at the below if the two *_icr_*() functions were moved > from 8250_port, how they have been used before? Dead code? They continue being used throughout 8250_port.c: `serial_icr_read' in `autoconfig_has_efr' and `serial_icr_write' in `serial8250_stop_tx', `__start_tx', and `serial8250_do_startup'. If they were dead, GCC would complain about their presence (without `__maybe_unused' annotation or an equivalent syntax to get the `unused' function atttribute). Now `serial_icr_write' is also used by `pci_oxsemi_tornado_set_divisor', and for consistency I chose to export `serial_icr_read' as well. Let me know if these explanations have cleared your concerns. > > + /* Old custom speed handling. */ > > + if (baud == 38400 && (port->flags & UPF_SPD_MASK) == UPF_SPD_CUST) { > > This part is not needed. We have a BOTHER mechanism in the kernel > that works for each driver that supports it in the generic way, hence > the user space tool wouldn't be patched to support this exact card > separately. Using SPD_CUST is a step back. As previously discussed I maintain this is a reasonable compromise until we have issues with BOTHER and other regular termios interfaces such as B200 fixed; specifically the 16-bit UART_DIV_MAX limitation making it impossible to request baud rates below 300bps, as also noted in the change description, and the inability to set exact clock rates which may be required in some applications relying on the presence of the SPD_CUST feature. NB I agree that wiring SPD_CUST to the 38400bps baud rate hasn't been particularly fortunate, but that's what used to be available in terms of the API back in 1990s, as I previously explained. I have now posted v5 with the grammatical issues fixed and an additional update to make use of the DIV_ROUND_CLOSEST macro I have come across while looking into an unrelated issue. Thank you for your review. Maciej