[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <b0c227b4-9233-4743-9c4e-0d775d4047bc@hammernet.be>
Date: Fri, 12 Sep 2025 15:57:20 +0200
From: Hendrik Hamerlinck <hendrik.hamerlinck@...mernet.be>
To: Yixun Lan <dlan@...too.org>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
paul.walmsley@...ive.com, palmer@...belt.com, aou@...s.berkeley.edu,
alex@...ti.fr, 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] riscv: dts: spacemit: add UART pinctrl combinations
Hello Yixun,
Thank you for reviewing.
On 9/11/25 13:22, Yixun Lan wrote:
> Hi Hendrik,
>
> On 16:53 Wed 03 Sep , Hendrik Hamerlinck wrote:
>> This adds 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.
>>
>> For convenience, the board DTS files have been updated to include all
>> UART instances with their possible pinmux options in a disabled state.
>>
>> Tested this locally on both Orange Pi RV2 and Banana Pi BPI-F3 boards.
>>
>> Signed-off-by: Hendrik Hamerlinck <hendrik.hamerlinck@...mernet.be>
>> ---
>> .../boot/dts/spacemit/k1-bananapi-f3.dts | 18 ++
>> .../boot/dts/spacemit/k1-orangepi-rv2.dts | 18 ++
>> arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi | 276 +++++++++++++++++-
>> 3 files changed, 309 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
>> index 6013be258542..661d47d1ce9e 100644
>> --- a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
>> +++ b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
>> @@ -49,3 +49,21 @@ &uart0 {
>> pinctrl-0 = <&uart0_2_cfg>;
>> status = "okay";
>> };
>> +
>> +&uart5 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&uart5_3_cfg>;
>> + status = "disabled";
>> +};
>> +
>> +&uart8 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&uart8_2_cfg>;
>> + status = "disabled";
>> +};
>> +
>> +&uart9 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&uart9_2_cfg>;
>> + status = "disabled";
>> +};
> all of uart5, 8, 9 come from 26-pins port, the functionaly is
> very likely depending on the final use case.. and I get your idea
> of adding those nodes but with "disabled" status..
>
> my suggestion is to not add them, or leave to users to add separated
> dtbo (Device tree overlays) files in the future
Fair enough, I was already doubting adding them. Most other .dts files
don't include them either. I’ll remove them in the next version.
>
> but I'm ok to complete uart pinctrl info in the dtsi file
>
>> diff --git a/arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts b/arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts
>> index 337240ebb7b7..dc45b75b1ad4 100644
>> --- a/arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts
>> +++ b/arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts
>> @@ -38,3 +38,21 @@ &uart0 {
>> pinctrl-0 = <&uart0_2_cfg>;
>> status = "okay";
>> };
>> +
>> +&uart5 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&uart5_3_cfg>;
>> + status = "disabled";
>> +};
>> +
>> +&uart8 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&uart8_2_cfg>;
>> + status = "disabled";
>> +};
>> +
>> +&uart9 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&uart9_2_cfg>;
>> + status = "disabled";
>> +};
>> diff --git a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
>> index 381055737422..43425530b5bf 100644
>> --- a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
>> +++ b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
>> @@ -11,12 +11,282 @@
>> #define K1_GPIO(x) (x / 32) (x % 32)
>>
>> &pinctrl {
>> + uart0_0_cfg: uart0-0-cfg {
>> + uart0-0-pins {
>> + pinmux = <K1_PADCONF(104, 3)>, /* uart0_txd */
>> + <K1_PADCONF(105, 3)>; /* uart0_rxd */
>> + power-source = <3300>;
>> + bias-pull-up;
>> + drive-strength = <19>;
>> + };
>> + };
>> +
>> + uart0_1_cfg: uart0-1-cfg {
>> + uart0-1-pins {
>> + pinmux = <K1_PADCONF(108, 1)>, /* uart0_txd */
>> + <K1_PADCONF(80, 3)>; /* uart0_rxd */
>> + power-source = <3300>;
>> + bias-pull-up;
>> + drive-strength = <19>;
>> + };
>> + };
>> +
>> uart0_2_cfg: uart0-2-cfg {
>> uart0-2-pins {
>> - pinmux = <K1_PADCONF(68, 2)>,
>> - <K1_PADCONF(69, 2)>;
>> + pinmux = <K1_PADCONF(68, 2)>, /* uart0_txd */
>> + <K1_PADCONF(69, 2)>; /* uart0_rxd */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>>
>> - bias-pull-up = <0>;
>> + uart2_0_cfg: uart2-0-cfg {
>> + uart2-0-pins {
>> + pinmux = <K1_PADCONF(21, 1)>, /* uart2_txd */
>> + <K1_PADCONF(22, 1)>, /* uart2_rxd */
>> + <K1_PADCONF(23, 1)>, /* uart2_cts */
>> + <K1_PADCONF(24, 1)>; /* uart2_rts */
> I think for group has cts, rts pins, it's more practical to
> have two separated cfgs, so the final application can choose to
> request two pins (tx, rx), or four pins (tx, tx, cts, rts)..
> (I believe the hardware should support this)
>
> something like this:
>
> uart2_0_cfg: uart2-0-cfg {
> uart2-0-pins {
> pinmux = <K1_PADCONF(21, 1)>, /* uart2_txd */
> <K1_PADCONF(22, 1)>, /* uart2_rxd */
> };
> };
>
> 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 */
> };
> };
>
> &uart2 {
> pinctrl-names = "default";
> pinctrl-0 = <&uart2_0_cfg>, <&uart2_0_cts_rts_cfg>;
> };
This sounds like good idea. There were some weird pin sequences, listing
them that way would result in a better structure (f.e. uart9_1_cfg).
The hardware seems to deal with it just fine.
I will update it this way in the next version.
>
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart3_0_cfg: uart3-0-cfg {
>> + uart3-0-pins {
>> + pinmux = <K1_PADCONF(81, 2)>, /* uart3_txd */
>> + <K1_PADCONF(82, 2)>, /* uart3_rxd */
>> + <K1_PADCONF(83, 2)>, /* uart3_cts */
>> + <K1_PADCONF(84, 2)>; /* uart3_rts */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart3_1_cfg: uart3-1-cfg {
>> + uart3-1-pins {
>> + pinmux = <K1_PADCONF(18, 2)>, /* uart3_txd */
>> + <K1_PADCONF(19, 2)>, /* uart3_rxd */
>> + <K1_PADCONF(20, 2)>, /* uart3_cts */
>> + <K1_PADCONF(21, 2)>; /* uart3_rts */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart3_2_cfg: uart3-2-cfg {
>> + uart3-2-pins {
>> + pinmux = <K1_PADCONF(53, 4)>, /* uart3_txd */
>> + <K1_PADCONF(54, 4)>, /* uart3_rxd */
>> + <K1_PADCONF(55, 4)>, /* uart3_cts */
>> + <K1_PADCONF(56, 4)>; /* uart3_rts */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart4_0_cfg: uart4-0-cfg {
>> + uart4-0-pins {
>> + pinmux = <K1_PADCONF(100, 4)>, /* uart4_txd */
>> + <K1_PADCONF(101, 4)>; /* uart4_rxd */
>> + power-source = <3300>;
>> + bias-pull-up;
>> + drive-strength = <19>;
>> + };
>> + };
>> +
>> + uart4_1_cfg: uart4-1-cfg {
>> + uart4-1-pins {
>> + pinmux = <K1_PADCONF(81, 3)>, /* uart4_cts */
>> + <K1_PADCONF(82, 3)>, /* uart4_rts */
>> + <K1_PADCONF(83, 3)>, /* uart4_txd */
>> + <K1_PADCONF(84, 3)>; /* uart4_rxd */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart4_2_cfg: uart4-2-cfg {
>> + uart4-2-pins {
>> + pinmux = <K1_PADCONF(23, 2)>, /* uart4_txd */
>> + <K1_PADCONF(24, 2)>; /* uart4_rxd */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart4_3_cfg: uart4-3-cfg {
>> + uart4-3-pins {
>> + pinmux = <K1_PADCONF(33, 2)>, /* uart4_txd */
>> + <K1_PADCONF(34, 2)>, /* uart4_rxd */
>> + <K1_PADCONF(35, 2)>, /* uart4_cts */
>> + <K1_PADCONF(36, 2)>; /* uart4_rts */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart4_4_cfg: uart4-4-cfg {
>> + uart4-4-pins {
>> + pinmux = <K1_PADCONF(111, 4)>, /* uart4_txd */
>> + <K1_PADCONF(112, 4)>, /* uart4_rxd */
>> + <K1_PADCONF(113, 4)>, /* uart4_cts */
>> + <K1_PADCONF(114, 4)>; /* uart4_rts */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart5_0_cfg: uart5-0-cfg {
>> + uart5-0-pins {
>> + pinmux = <K1_PADCONF(102, 3)>, /* uart5_txd */
>> + <K1_PADCONF(103, 3)>; /* uart5_rxd */
>> + power-source = <3300>;
>> + bias-pull-up;
>> + drive-strength = <19>;
>> + };
>> + };
>> +
>> + uart5_1_cfg: uart5-1-cfg {
>> + uart5-1-pins {
>> + pinmux = <K1_PADCONF(25, 2)>, /* uart5_txd */
>> + <K1_PADCONF(26, 2)>, /* uart5_rxd */
>> + <K1_PADCONF(27, 2)>, /* uart5_cts */
>> + <K1_PADCONF(28, 2)>; /* uart5_rts */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart5_2_cfg: uart5-2-cfg {
>> + uart5-2-pins {
>> + pinmux = <K1_PADCONF(42, 2)>, /* uart5_txd */
>> + <K1_PADCONF(43, 2)>, /* uart5_rxd */
>> + <K1_PADCONF(44, 2)>, /* uart5_cts */
>> + <K1_PADCONF(45, 2)>; /* uart5_rts */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart5_3_cfg: uart5-3-cfg {
>> + uart5-3-pins {
>> + pinmux = <K1_PADCONF(70, 4)>, /* uart5_txd */
>> + <K1_PADCONF(71, 4)>, /* uart5_rxd */
>> + <K1_PADCONF(72, 4)>, /* uart5_cts */
>> + <K1_PADCONF(73, 4)>; /* uart5_rts */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart6_0_cfg: uart6-0-cfg {
>> + uart6-0-pins {
>> + pinmux = <K1_PADCONF(85, 2)>, /* uart6_cts */
>> + <K1_PADCONF(86, 2)>, /* uart6_txd */
>> + <K1_PADCONF(87, 2)>, /* uart6_rxd */
>> + <K1_PADCONF(90, 2)>; /* uart6_rts */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart6_1_cfg: uart6-1-cfg {
>> + uart6-1-pins {
>> + pinmux = <K1_PADCONF(0, 2)>, /* uart6_txd */
>> + <K1_PADCONF(1, 2)>, /* uart6_rxd */
>> + <K1_PADCONF(2, 2)>, /* uart6_cts */
>> + <K1_PADCONF(3, 2)>; /* uart6_rts */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart6_2_cfg: uart6-2-cfg {
>> + uart6-2-pins {
>> + pinmux = <K1_PADCONF(56, 2)>, /* uart6_txd */
>> + <K1_PADCONF(57, 2)>; /* uart6_rxd */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart7_0_cfg: uart7-0-cfg {
>> + uart7-0-pins {
>> + pinmux = <K1_PADCONF(88, 2)>, /* uart7_txd */
>> + <K1_PADCONF(89, 2)>; /* uart7_rxd */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart7_1_cfg: uart7-1-cfg {
>> + uart7-1-pins {
>> + pinmux = <K1_PADCONF(4, 2)>, /* uart7_txd */
>> + <K1_PADCONF(5, 2)>, /* uart7_rxd */
>> + <K1_PADCONF(6, 2)>, /* uart7_cts */
>> + <K1_PADCONF(7, 2)>; /* uart7_rts */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart8_0_cfg: uart8-0-cfg {
>> + uart8-0-pins {
>> + pinmux = <K1_PADCONF(82, 4)>, /* uart8_txd */
>> + <K1_PADCONF(83, 4)>; /* uart8_rxd */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart8_1_cfg: uart8-1-cfg {
>> + uart8-1-pins {
>> + pinmux = <K1_PADCONF(8, 2)>, /* uart8_txd */
>> + <K1_PADCONF(9, 2)>, /* uart8_rxd */
>> + <K1_PADCONF(10, 2)>, /* uart8_cts */
>> + <K1_PADCONF(11, 2)>; /* uart8_rts */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart8_2_cfg: uart8-2-cfg {
>> + uart8-2-pins {
>> + pinmux = <K1_PADCONF(75, 4)>, /* uart8_txd */
>> + <K1_PADCONF(76, 4)>, /* uart8_rxd */
>> + <K1_PADCONF(77, 4)>, /* uart8_cts */
>> + <K1_PADCONF(78, 4)>; /* uart8_rts */
>> + power-source = <3300>;
>> + bias-pull-up;
>> + drive-strength = <19>;
>> + };
>> + };
>> +
>> + uart9_0_cfg: uart9-0-cfg {
>> + uart9-0-pins {
>> + pinmux = <K1_PADCONF(12, 2)>, /* uart9_txd */
>> + <K1_PADCONF(13, 2)>; /* uart9_rxd */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart9_1_cfg: uart9-1-cfg {
>> + uart9-1-pins {
>> + pinmux = <K1_PADCONF(110, 3)>, /* uart9_cts */
>> + <K1_PADCONF(115, 3)>, /* uart9_rts */
>> + <K1_PADCONF(116, 3)>, /* uart9_txd */
>> + <K1_PADCONF(117, 3)>; /* uart9_rxd */
>> + bias-pull-up;
>> + drive-strength = <32>;
>> + };
>> + };
>> +
>> + uart9_2_cfg: uart9-2-cfg {
>> + uart9-2-pins {
>> + pinmux = <K1_PADCONF(72, 2)>, /* uart9_txd */
>> + <K1_PADCONF(73, 2)>; /* uart9_rxd */
>> + bias-pull-up;
>> drive-strength = <32>;
>> };
>> };
>> --
>> 2.43.0
>>
Kind regards,
Hendrik
Powered by blists - more mailing lists