Re: [PATCH v4] riscv: dts: spacemit: add UART pinctrl combinations

[Yixun Lan <dlan@gentoo.org>]

Hi Troy,

On 16:42 Wed 17 Sep     , Troy Mitchell wrote:
> On Wed, Sep 17, 2025 at 10:17:16AM +0200, Hendrik Hamerlinck wrote:
> > Hello Troy,
> > 
> > Thank you for reviewing.
> > 
> > On 9/17/25 09:46, Troy Mitchell wrote:
> > > Hi Hendrik, Thanks for your patch!
> > >
> > > On Wed, Sep 17, 2025 at 08:59:07AM +0200, Hendrik Hamerlinck wrote:
> > >> Add UART pinctrl configurations based on the SoC datasheet and the
> > >> downstream Bianbu Linux tree. The drive strength values were taken from
> > >> the downstream implementation, which uses medium drive strength.
> > >> CTS/RTS are moved to separate *-cts-rts-cfg states so boards can enable
> > >> hardware flow control conditionally.
> > >>
> > >> Signed-off-by: Hendrik Hamerlinck <hendrik.hamerlinck@hammernet.be>
> > >> Reviewed-by: Yixun Lan <dlan@gentoo.org>
> > >> ---
> > >> Changes in v4:
> > >> - Explicitly use 0 as bias-pull-up value
> > >>
> > >> Changes in v3:
> > >> - Added /omit-if-no-ref/ to pinctrl states to reduce DT size
> > >>
> > >> Changes in v2:
> > >> - Split cts/rts into separate pinctrl configs as suggested
> > >> - Removed options from board DTS files to keep them cleaner
> > >> ---
> > >>  arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi | 430 ++++++++++++++++++-
> > >>  1 file changed, 428 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> > >> +	/omit-if-no-ref/
> > >> +	uart2_0_cts_rts_cfg: uart2-0-cts-rts-cfg {
> > >> +		uart2-0-pins {
> > >> +			pinmux = <K1_PADCONF(23, 1)>,	/* uart2_cts */
> > >> +				 <K1_PADCONF(24, 1)>;	/* uart2_rts */
> > >> +			bias-pull-up = <0>;
> > >> +			drive-strength = <32>;
> > >> +		};
> > >> +	};
> > > We are currently using the 8250 UART driver, but hardware flow control does not
> > > work properly on K1. We are considering fixing this when time permits.
> > >
> > > Should we add a comment here to avoid confusing others who may run into this?
> > > If so, I will remove the comment once the issue has been fixed.
> > Not sure if I’m missing something, but hardware flow control seems to work
> > with the `8250_of` driver?
> > 
> > On both ends I configured the ports with:
> > `stty -F /dev/ttyS1 115200 crtscts -ixon -ixoff raw -echo`
> > 
> > With this setup, data sent with echo only goes through when the peer is
> > actually reading on the other side, which looks like RTS/CTS is
> > functioning correctly.
> Let me clarify my earlier reply(I have double checked):
> It is not that hardware flow control does not work at all,
> but rather that it fails intermittently.
> The higher the baud rate, the sooner the problem tends to show up.
> 
> From your log I noticed the baud rate is 115200.
> At this rate, it might take a full day of continuous testing before
> the issue appears (though of course it could also fail much sooner, since it is probabilistic).
> 
> So I am wondering whether we should add a comment here,
> or instead put the note in the UART node.
No, please start another thread to address this issue, find a solution
and then ideally work out a patch for it, adding comment doesn't really help.

Besides, what Hendrik doing here is to complete the descriptions of UART pinctrl
which is a different thing, let's not mix them together.

-- 
Yixun Lan (dlan)