On 20/12/2022 10.38, Greg Kroah-Hartman wrote:
> On Tue, Dec 20, 2022 at 10:16:38AM +0100, Rasmus Villemoes wrote:
>> Hi,
>>
>> I think something went slightly wrong when 7c7f9bc98 (serial: Deassert
>> Transmit Enable on probe in driver-specific way) got backported to
>> 5.15.y. In fsl_lpuart.c, the original had this
>>
>> failed_irq_request:
>> -failed_get_rs485:
>> uart_remove_one_port(&lpuart_reg, &sport->port);
>> failed_attach_port:
>> +failed_get_rs485:
>> failed_reset:
>> lpuart_disable_clks(sport);
>> return ret;
>>
>> in the error path, but that is missing in the backport. So if we now hit
>> the 'goto failed_get_rs485;', we'll do uart_remove_one_port() while
>> uart_add_one_port() hasn't been done.
>
> Ick, can you send a patch to fix this up?
Done.
But a colleague just spotted another difference. In uart_suspend_port()
in serial_core.c, this hunk
spin_lock_irq(&uport->lock);
ops->stop_tx(uport);
- ops->set_mctrl(uport, 0);
+ if (!(uport->rs485.flags & SER_RS485_ENABLED))
+ ops->set_mctrl(uport, 0);
/* save mctrl so it can be restored on resume */
mctrl = uport->mctrl;
uport->mctrl = 0;
ops->stop_rx(uport);
spin_unlock_irq(&uport->lock);
is also missing. The context is different; the /* save mctrl... */
comment doesn't exist in 5.15.y, but was added in commit 18c9d4a3c2 (5.19).
I don't know if 18c9d4a3c2 should also be backported - the description
does sound like it fixes a real bug. But probably the ->set_mctrl()
should be guarded by the !rs485 regardless.
Rasmus
