Re: [PATCH 3/3] serial: 8250_dw: Add Allwinner A733 UART

[Vinicius Pedrosa <vinicius.eduardo.pedrosa@gmail.com>]

Hi Andre,

On 10/6/26 14:16, Andre Przywara wrote:
> So isn't that a bug somewhere else then? Why would it gate the clock to
> set the rate?

I think the bug is in dw8250_set_termios(). The gate comes from
4e26b134bd17 (2014, replacing the Baytrail hook) and runs on every
termios change that reaches the driver, with nothing draining the port
first. On LPSS the UART's clock ends in a gate on bit 31, the divider
update bit, so I'd guess the disable/enable pair is how a new rate gets
latched there. On the A733 the DT hands over only the bus gate, so that
is what gets pulsed.

> And what is your use case here, exactly? Do you reconfigure the baud
> rate while characters are incoming?

No baud change. I toggled only CLOCAL with TCSANOW, 300 times while the
console printed 3000 lines. With the gate alone every run lost 12 to 18
lines (3 runs). With round_rate and set_rate but no gate, all 3 were
clean. A CSTOPB toggle splits the same way over 6 runs each.

> So can you just expose CLK_APB1 as the baud rate clock in the DT?

On the A733 the UARTs aren't on APB1. They sit on APB_UART (0x538),
shared by the seven UARTs. I tried it as baudclk with the bus gate as
apb_pclk and the console was garbage. The driver asks for 16 x baud and
the CCU rounds down, so 115200 gets RC16M/9 = 1777778 and divisor 1
gives 111111 baud. 24M/13 is just above the target, so it isn't picked.
ccu-sun6i-rtc lists RC16M at 30% accuracy anyway.

It's also shared: 01569a29af76 removed the clock notifier in 7.2, so a
port that re-rates APB_UART would leave the other six with stale
divisors.

For v2 I'd drop SKIP_SET_RATE for the A733 and only gate and set the
rate when clk_round_rate() differs from clk_get_rate(). By my reading
the branch is never taken here, and that kernel was clean in all 13
runs. H6, H616 and A523 pass only a bus gate too, so they likely see the
same thing, but I can't test them.

Would you rather drop the gate altogether? I kept it because I can't
test LPSS.

Thanks,
Vinicius