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

[Bryan O'Donoghue]

On 03/09/2023 14:02, Ziyang Huang wrote:
> 在 2023/9/1 23:04, Bryan O'Donoghue 写道:
> > <...>
> >
> > The assignment of pins 20 and 21 to blsp1_uart1 is not correct.
> >
> > The blspX_uartY in pinctrl should match what is in the dtsi so 
> > assigning pins_a above to blsp1_uart1 is not right. The dts name and 
> > pinctrl name should be the same.
> >
> > Your console is on blsp0_uart0.
> >
> > https://git.codelinaro.org/clo/qsdk/oss/boot/u-boot-2016/-/blob/5343739b4070bcec2fecd72f758c16adc31a3083/arch/arm/dts/ipq5018-mp03.3.dts#L33
> >
> > So roughly speaking
> >
> > arch/arm64/boot/dts/qcom/ipq5018-rdp432-c2.dts
> >
> > aliases {
> >      serial0 = &blsp0_uart0;
> > };
> >
> > chosen {
> >      stdout-path = "serial0:115200n8";
> > };
> >
> > &blsp0_uart0 {
> >          pinctrl-0 = <&uart0_pins>;
> >          pinctrl-names = "default";
> >          status = "okay";
> > };
> >
> >
> > arch/arm64/boot/dts/qcom/ipq5018.dtsi
> >
> > blsp0_uart0: serial@78af000
> >
> > either that or  blsp0_uart1 for pins28 and pins29 - you seem to 
> > indicate pins_1 => blsp0_uart0.
> >
> > The two roots of the problem are
> >
> > 1. Mislabeling of the uart block in the dtsi
> > 2. Invalid miscongiruation of pins for that misnamed block
> >
> > The fix should be
> >
> > 1. Fix the labeling of uart in the dtsi
> > 2. Decide on which pins gpio20, gpio21 ? are the right ones to configure
> >
> > I thought you said in a previous email if you changed pins gpio28 and 
> > gpio29 that the UART would fail if so that implies blsp0_uart1.
> >
> > Either way the pinctrl and dts should agree.
> >
> > ---
> > bod
>
> No, please read my commit message carefully.
>
> The Y of pinctrl is the index of pinmux config. So it can't be used in 
> the serial node definition.
>
> Please note that the physical port of first serial is configurable. It 
> can use gpio20, gpio21 or/and gpio28,29. All of these pins are for the 
> first serial.
>
> Let's take the second serial as an example. It has 3 configurable 
> physical port groups - "blsp1_uart0" (pinconfig name, use GPIO 
> 10,11,12,13), "blsp1_uart1" (gpio 31,32,33,34), "blsp1_uart2" (gpio 
> 23,24,25,26).
>
> But the dts name of the second serial definition is "blsp1_uart2". 
> Because it the second serial of the first BLSP block.
>
> Same logic. The dts name of the first serial definition is 
> "blsp1_uart1". Because it the first serial of the first BLSP block.
>
> I think I need to introduce the architecture of these SoC. It has two 
> BLSP block. Each BLSP block has several uart port.
>
> So the dts name of serial contains the BLSP index and the serial index 
> inside BLSP. But pinconf name doesn't care about it. So it use global 
> index. And due to the physical ports are configurable, it need pinmux 
> index.
>
> The equation will be like this:
>
> dts name of serial definition: "blspX_uartY"
> pinconf name: "blspU_uartV"
> U = (uart_number_inside_each_blsp * (X - 1)) + (Y - 1)

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