Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> writes: > On Fri, Jul 26, 2019 at 09:52:41PM +0300, Sergey Organov wrote: >> Called in only one place, for RS232, it only obscures things, as it >> doesn't go well with 2 similar named functions, >> imx_uart_rts_inactive() and imx_uart_rts_active(), that both are >> RS485-specific. > > I don't share the critic. IMHO the name is fine. imx_uart_rts_inactive > sets rts to its inactive level, > imx_uart_rts_active() to its active level Not exactly, in fact both do more than that, in a similar manner. > imx_uart_rts_auto() lets the output drive automatically by the > receiver. And this one was different and it was rather confusing when I've tried to grok the logic of the driver. > The name started to be a bit wrong in patch 1 of the series however. The function was different from first two even before the patch, as it does not do any of those additional things the first two do. > And I still object removing this function because with the semantic > this function got in patch 1 it is suiteable to be used in > imx_uart_set_mctrl(). It is not, as it does require change to be used there, as we've already seen, and then it becomes very different function from what it was at the beginning. Even then, the end result I've shown you when attempting to somehow preserve some re-incarnation of this function still seems more cumbersome to me than the end result of these patches. That said, this a matter of taste and style, not correctness, and could be changed as a follow-up, not to risk breaking already tested patch series. Thanks, -- Sergey
