2023-09-05 16:22:51

by Ziyang Huang

[permalink] [raw]
Subject: [PATCH v3] arm64: dts: ipq5018: Correct uart1_pins pinconf

In pinctrl, the pinconfigs for uart are named "blspX_uartY".
X is the UART ID. Starts from 1.
1-6 are in BLSP Block 1.
7-12 are in BLSP Block 2.
Y is the index of mux config. Starts from 0.

In dts, the serials are also named "blspX_uartY", but with different logic.
X is the BLSP Block ID. Starts from 1.
Y is the uart id inside block.
In "ipq6018.dtsi" and "ipq8074.dtsi", it starts from 1.
But in "ipq5332.dtsi" and "ipq9574.dtsi", it starts from 0.

+-----------------+-----------------+-------------+-----------------+
| Block ID | ID inside Block | dts name | pinconfig name |
| (Starts from 1) | (Starts from 1) | | |
+-----------------+-----------------+-------------+-----------------+
| 1 | 1 | blsp1_uart1 | blsp0_uartY |
| 1 | 2 | blsp1_uart2 | blsp1_uartY |
| 1 | 6 | blsp1_uart6 | blsp5_uartY |
| 2 | 1 | blsp2_uart1 | blsp6_uartY |
| 2 | 6 | blsp2_uart6 | blsp12_uartY |
+-----------------+-----------------+-------------+-----------------+

In "ipq5018.dts", "blsp1_uart1" (dts name) is the first serial (confimed
by the address), So its pinconfig should be "blsp0_uart0" (pinconfig name,
use GPIO 20 and 21) or "blsp0_uart1" (pinconfig name, use GPIO 28 and 29).

Fixes: 570006756a16 ("arm64: dts: Add ipq5018 SoC and rdp432-c2 board support")
Signed-off-by: Ziyang Huang <[email protected]>
---
Changes since v1:
- Use corrent name in From

Changes since v2:
- Define 2 pinconfs for uart1 in ipq5018.dtsi
- rdp432-c2 use uart1_pins_a

arch/arm64/boot/dts/qcom/ipq5018-rdp432-c2.dts | 2 +-
arch/arm64/boot/dts/qcom/ipq5018.dtsi | 15 +++++++++++----
2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq5018-rdp432-c2.dts b/arch/arm64/boot/dts/qcom/ipq5018-rdp432-c2.dts
index e636a1cb9b77..e83d1863e89c 100644
--- a/arch/arm64/boot/dts/qcom/ipq5018-rdp432-c2.dts
+++ b/arch/arm64/boot/dts/qcom/ipq5018-rdp432-c2.dts
@@ -23,7 +23,7 @@ chosen {
};

&blsp1_uart1 {
- pinctrl-0 = <&uart1_pins>;
+ pinctrl-0 = <&uart1_pins_a>;
pinctrl-names = "default";
status = "okay";
};
diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
index 9f13d2dcdfd5..50b4a2bd6fd3 100644
--- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
@@ -103,11 +103,18 @@ tlmm: pinctrl@1000000 {
interrupt-controller;
#interrupt-cells = <2>;

- uart1_pins: uart1-state {
- pins = "gpio31", "gpio32", "gpio33", "gpio34";
- function = "blsp1_uart1";
+ uart1_pins_a: uart1@0 {
+ pins = "gpio20", "gpio21";
+ function = "blsp0_uart0";
drive-strength = <8>;
- bias-pull-down;
+ bias-disabled;
+ };
+
+ uart1_pins_b: uart1@1 {
+ pins = "gpio28", "gpio29";
+ function = "blsp0_uart1";
+ drive-strength = <8>;
+ bias-disabled;
};
};

--
2.40.1


2023-09-13 00:57:33

by Ziyang Huang

[permalink] [raw]
Subject: Re: [PATCH v3] arm64: dts: ipq5018: Correct uart1_pins pinconf

在 2023/9/4 8:57, Bryan O'Donoghue 写道:
> <...>
>
> I've checked the documentation for this chip.
>
> gpio20, gpio21 = blsp0_uart0
> gpio28, gpio29 = blsp0_uart0
>
> These pins are muxed to UART0, I agree, the u-boot dts also indicates
> this also.
>
> If we open the documentation further we see
>
> 0x78AF000 = BLSP1_BLSP_UART0
> 0x79b0000 = BLSP1_BLSP_UART1
>
> So for starters the dtsi has the _wrong_ label.
>
> Here/anseo
>
> grep uart0: arch/arm64/boot/dts/qcom/*
> arch/arm64/boot/dts/qcom/ipq5332.dtsi:        blsp1_uart0: serial@78af000 {
> arch/arm64/boot/dts/qcom/ipq9574.dtsi:        blsp1_uart0: serial@78af000 {
>
> That's how that label ought to be the main hint something is askance is
> assigning a pin named "blsp0_uart0" to a dts entry named "blsp1_uart1".
>
> Please update the label in your next revision.
>
> ---
> bod

I think the root cause is the confused name in pinctrl. I will update
the mux index to alphabetical order in next patch.

By the way, can you find out the documents about the pinmux map. For
example, the code of pinctrl only show that GPIO20,21 are for UART0. But
which pin is TX and which is RX? And yes, because of UART, it's easy to
find out.

But what I want to known is "blsp2_spi". It has 3 pinmux configs -
"blsp2_spi" (GPIO27), "blsp2_spi0" (GPIO31,32,33,34) and "blsp2_spi1"
(GPIO23,24,25,26). What "blsp2_spi" (GPIO27) for?