[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <B8941D240B018103+aMp69Tod8hNbLcu_@LT-Guozexi>
Date: Wed, 17 Sep 2025 17:10:13 +0800
From: Troy Mitchell <troy.mitchell@...ux.spacemit.com>
To: Yixun Lan <dlan@...too.org>,
Troy Mitchell <troy.mitchell@...ux.spacemit.com>
Cc: Hendrik Hamerlinck <hendrik.hamerlinck@...mernet.be>,
paul.walmsley@...ive.com, palmer@...belt.com, alex@...ti.fr,
aou@...s.berkeley.edu, conor+dt@...nel.org, krzk+dt@...nel.org,
robh@...nel.org, skhan@...uxfoundation.org,
linux-kernel-mentees@...ts.linux.dev, devicetree@...r.kernel.org,
linux-riscv@...ts.infradead.org, spacemit@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] riscv: dts: spacemit: add UART pinctrl combinations
On Wed, Sep 17, 2025 at 05:05:32PM +0800, Yixun Lan wrote:
> 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@...mernet.be>
> > > >> Reviewed-by: Yixun Lan <dlan@...too.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.
Yes, we already have a solution. If I can spare some time soon,
I will clean it up and post it ASAP.
Thanks for the reminder, Yixun. Indeed, adding a comment doesn’t really help here.
>
> Besides, what Hendrik doing here is to complete the descriptions of UART pinctrl
> which is a different thing, let's not mix them together.
Yes, my rb is valid.
- Troy
>
> --
> Yixun Lan (dlan)
>
Powered by blists - more mailing lists