Add QSPI and QUPv3 DT nodes for SC7280 SoC
Subsystems (4)
v4 cover Add QSPI and QUPv3 DT nodes for SC7280 SoC
Roja Rani Yarubandi (4):
arm64: dts: sc7280: Add QSPI node
arm64: dts: sc7280: Add QUPv3 wrapper_0 nodes
arm64: dts: sc7280: Update QUPv3 Debug UART DT node
arm64: dts: sc7280: Add QUPv3 wrapper_1 nodes
arch/arm64/boot/dts/qcom/sc7280-idp.dts | 133 +- arch/arm64/boot/dts/qcom/sc7280.dtsi | 3169 +++++++++++++++++++++++-------- 2 files changed, 2482 insertions(+), 820 deletions(-)
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH v4 1/4 arm64: dts: sc7280: Add QSPI node
From: Roja Rani Yarubandi <[email protected]>
Add QSPI DT node for SC7280 SoC.
---
Changes in V4:
- As per Stephen's comment updated spi-max-frequency to 37.5MHz, moved qspi_opp_table from /soc to / (root).
Changes in V3:
- Broken the huge V2 patch into 3 smaller patches.
1. QSPI DT nodes
2. QUP wrapper_0 DT nodes
3. QUP wrapper_1 DT nodes
Changes in V2:
- As per Doug's comments removed pinmux/pinconf subnodes.
- As per Doug's comments split of SPI, UART nodes has been done.
- Moved QSPI node before aps_smmu as per the order.
arch/arm64/boot/dts/qcom/sc7280-idp.dts | 27 ++++++++++++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 62 +++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+)
arch/arm64/boot/dts/qcom/sc7280-idp.dts +27 -0
@@ -269,6 +269,20 @@ }; }; +&qspi { + status = "okay"; + pinctrl-names = "default"; + pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>; + + flash@0 { + compatible = "jedec,spi-nor"; + reg = <0>; + spi-max-frequency = <37500000>; + spi-tx-bus-width = <2>; + spi-rx-bus-width = <2>; + }; +}; + &qupv3_id_0 { status = "okay"; };
@@ -346,6 +360,19 @@ /* PINCTRL - additions to nodes defined in sc7280.dtsi */ +&qspi_cs0 { + bias-disable; +}; + +&qspi_clk { + bias-disable; +}; + +&qspi_data01 { + /* High-Z when no transfers; nice to park the lines */ + bias-pull-up; +}; + &qup_uart5_default { tx { pins = "gpio46";
arch/arm64/boot/dts/qcom/sc7280.dtsi +62 -0
80 lines not on this page · raw message
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH V4 1/4] arm64: dts: sc7280: Add QSPI node
Quoted 58 lines · Rajesh Patil
From: Roja Rani Yarubandi <[email protected]>
Add QSPI DT node for SC7280 SoC.
Signed-off-by Roja Rani Yarubandi <[email protected]>Signed-off-by Rajesh Patil <[email protected]>---
Changes in V4:
- As per Stephen's comment updated spi-max-frequency to 37.5MHz, movedqspi_opp_tablefrom /soc to / (root).Changes in V3:
- Broken the huge V2 patch into 3 smaller patches.
1. QSPI DT nodes
2. QUPwrapper_0DT nodes
3. QUPwrapper_1DT nodesChanges in V2:
- As per Doug's comments removed pinmux/pinconf subnodes.
- As per Doug's comments split of SPI, UART nodes has been done.
- Moved QSPI node beforeaps_smmuas per the order.arch/arm64/boot/dts/qcom/sc7280-idp.dts | 27 ++++++++++++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 62 +++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+)arch/arm64/boot/dts/qcom/sc7280-idp.dts +17
-0@@ -269,6 +269,20 @@ }; }; +&qspi { + status = "okay"; + pinctrl-names = "default"; + pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>; + + flash@0 { + compatible = "jedec,spi-nor"; + reg = <0>; + spi-max-frequency = <37500000>; + spi-tx-bus-width = <2>; + spi-rx-bus-width = <2>; + }; +}; + &qupv3_id_0 { status = "okay"; };@@ -346,6 +360,19 @@ /* PINCTRL - additions to nodes defined in sc7280.dtsi */ +&qspi_cs0 { + bias-disable; +};
arch/arm64/boot/dts/qcom/sc7280-idp.dts line 375
Quoted 10 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dts +10
-0+ +&qspi_clk { + bias-disable; +}; + +&qspi_data01 { + /* High-Z when no transfers; nice to park the lines */ + bias-pull-up; +}; +
This configures the SPI flash of the SC7280 IDP board, which is neither mentioned in the subject nor the body of the commit message. IMO this should be split out into a separate patch.
Re: [PATCH V4 1/4] arm64: dts: sc7280: Add QSPI node
Quoted 67 lines · Matthias Kaehlcke
Quoted 66 lines · Rajesh Patil
From: Roja Rani Yarubandi <[email protected]>
Add QSPI DT node for SC7280 SoC.
Signed-off-by Roja Rani Yarubandi <[email protected]>Signed-off-by Rajesh Patil <[email protected]>---
Changes in V4:
- As per Stephen's comment updated spi-max-frequency to 37.5MHz,
moved
qspi_opp_tablefrom /soc to / (root).Changes in V3:
- Broken the huge V2 patch into 3 smaller patches.
1. QSPI DT nodes
2. QUPwrapper_0DT nodes
3. QUPwrapper_1DT nodesChanges in V2:
- As per Doug's comments removed pinmux/pinconf subnodes.
- As per Doug's comments split of SPI, UART nodes has been done.
- Moved QSPI node beforeaps_smmuas per the order.arch/arm64/boot/dts/qcom/sc7280-idp.dts | 27 ++++++++++++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 62 +++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+)arch/arm64/boot/dts/qcom/sc7280-idp.dts +23
-0@@ -269,6 +269,20 @@ }; }; +&qspi { + status = "okay"; + pinctrl-names = "default"; + pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>; + + flash@0 { + compatible = "jedec,spi-nor"; + reg = <0>; + spi-max-frequency = <37500000>; + spi-tx-bus-width = <2>; + spi-rx-bus-width = <2>; + }; +}; + &qupv3_id_0 { status = "okay"; };@@ -346,6 +360,19 @@ /* PINCTRL - additions to nodes defined in sc7280.dtsi */ +&qspi_cs0 { + bias-disable; +}; + +&qspi_clk { + bias-disable; +}; + +&qspi_data01 {
Quoted 8 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dts line 375
Quoted 4 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dts +4
-0+ /* High-Z when no transfers; nice to park the lines */ + bias-pull-up; +}; +This configures the SPI flash of the SC7280 IDP board, which is neither mentioned in the subject nor the body of the commit message. IMO this should be split out into a separate patch.
Okay.
PATCH v4 2/4 arm64: dts: sc7280: Add QUPv3 wrapper_0 nodes
In reply to Rajesh PatilFrom: Roja Rani Yarubandi <[email protected]>
Add QUPv3 wrapper_0 DT nodes for SC7280 SoC.
---
Changes in V4:
- As per Bjorn's comment, added QUP Wrapper_0 nodes other than debug-uart node
- Dropped interconnect votes for wrapper_0 node
Changes in V3:
- Broken the huge V2 patch into 3 smaller patches.
1. QSPI DT nodes
2. QUP wrapper_0 DT nodes
3. QUP wrapper_1 DT nodes
Changes in V2:
- As per Doug's comments removed pinmux/pinconf subnodes.
- As per Doug's comments split of SPI, UART nodes has been done.
- Moved QSPI node before aps_smmu as per the order.
arch/arm64/boot/dts/qcom/sc7280-idp.dts | 84 ++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 720 ++++++++++++++++++++++++++++++++ 2 files changed, 804 insertions(+)
arch/arm64/boot/dts/qcom/sc7280-idp.dts +84 -0
@@ -358,6 +358,16 @@ vdda18-supply = <&vreg_l1c_1p8>; }; +&uart7 { + status = "okay"; + + /delete-property/interrupts; + interrupts-extended = <&intc GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>, + <&tlmm 31 IRQ_TYPE_EDGE_FALLING>; + pinctrl-names = "default", "sleep"; + pinctrl-1 = <&qup_uart7_sleep_cts>, <&qup_uart7_sleep_rts>, <&qup_uart7_sleep_tx>, <&qup_uart7_sleep_rx>; +}; + /* PINCTRL - additions to nodes defined in sc7280.dtsi */ &qspi_cs0 {
@@ -428,3 +438,77 @@ bias-pull-up; }; }; +&qup_uart7_cts { + /* + * Configure a pull-down on CTS to match the pull of + * the Bluetooth module. + */ + bias-pull-down; +}; + +&qup_uart7_rts { + /* We'll drive RTS, so no pull */ + drive-strength = <2>; + bias-disable; +}; + +&qup_uart7_tx { + /* We'll drive TX, so no pull */ + drive-strength = <2>; + bias-disable; +}; + +&qup_uart7_rx { + /* + * Configure a pull-up on RX. This is needed to avoid + * garbage data when the TX pin of the Bluetooth module is + * in tri-state (module powered off or not driving the + * signal yet). + */ + bias-pull-up; +}; + +&tlmm { + qup_uart7_sleep_cts: qup-uart7-sleep-cts { + pins = "gpio28"; + function = "gpio"; + /* + * Configure a pull-down on CTS to match the pull of + * the Bluetooth module. + */ + bias-pull-down; + }; + + qup_uart7_sleep_rts: qup-uart7-sleep-rts { + pins = "gpio29"; + function = "gpio"; + /* + * Configure pull-down on RTS. As RTS is active low + * signal, pull it low to indicate the BT SoC that it + * can wakeup the system anytime from suspend state by + * pulling RX low (by sending wakeup bytes). + */ + bias-pull-down; + }; + + qup_uart7_sleep_tx: qup-uart7-sleep-tx { + pins = "gpio30"; + function = "gpio"; + /* + * Configure pull-up on TX when it isn't actively driven + * to prevent BT SoC from receiving garbage during sleep. + */ + bias-pull-up; + }; + + qup_uart7_sleep_rx: qup-uart7-sleep-rx { + pins = "gpio31"; + function = "gpio"; + /* + * Configure a pull-up on RX. This is needed to avoid + * garbage data when the TX pin of the Bluetooth module + * is floating which may cause spurious wakeups. + */ + bias-pull-up; + }; +};
arch/arm64/boot/dts/qcom/sc7280.dtsi +720 -0
751 lines not on this page · raw message
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH V4 2/4] arm64: dts: sc7280: Add QUPv3 wrapper_0 nodes
Quoted 116 lines · Rajesh Patil
From: Roja Rani Yarubandi <[email protected]>
Add QUPv3
wrapper_0DT nodes for SC7280 SoC.Signed-off-by Roja Rani Yarubandi <[email protected]>Signed-off-by Rajesh Patil <[email protected]>---
Changes in V4:
- As per Bjorn's comment, added QUPWrapper_0nodes other than debug-uart node
- Dropped interconnect votes forwrapper_0nodeChanges in V3:
- Broken the huge V2 patch into 3 smaller patches.
1. QSPI DT nodes
2. QUPwrapper_0DT nodes
3. QUPwrapper_1DT nodesChanges in V2:
- As per Doug's comments removed pinmux/pinconf subnodes.
- As per Doug's comments split of SPI, UART nodes has been done.
- Moved QSPI node beforeaps_smmuas per the order.arch/arm64/boot/dts/qcom/sc7280-idp.dts | 84 ++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 720 ++++++++++++++++++++++++++++++++ 2 files changed, 804 insertions(+)arch/arm64/boot/dts/qcom/sc7280-idp.dts +74
-0@@ -358,6 +358,16 @@ vdda18-supply = <&vreg_l1c_1p8>; }; +&uart7 { + status = "okay"; + + /delete-property/interrupts; + interrupts-extended = <&intc GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>, + <&tlmm 31 IRQ_TYPE_EDGE_FALLING>; + pinctrl-names = "default", "sleep"; + pinctrl-1 = <&qup_uart7_sleep_cts>, <&qup_uart7_sleep_rts>, <&qup_uart7_sleep_tx>, <&qup_uart7_sleep_rx>; +}; + /* PINCTRL - additions to nodes defined in sc7280.dtsi */ &qspi_cs0 {@@ -428,3 +438,77 @@ bias-pull-up; }; }; +&qup_uart7_cts { + /* + * Configure a pull-down on CTS to match the pull of + * the Bluetooth module. + */ + bias-pull-down; +}; + +&qup_uart7_rts { + /* We'll drive RTS, so no pull */ + drive-strength = <2>; + bias-disable; +}; + +&qup_uart7_tx { + /* We'll drive TX, so no pull */ + drive-strength = <2>; + bias-disable; +}; + +&qup_uart7_rx { + /* + * Configure a pull-up on RX. This is needed to avoid + * garbage data when the TX pin of the Bluetooth module is + * in tri-state (module powered off or not driving the + * signal yet). + */ + bias-pull-up; +}; + +&tlmm { + qup_uart7_sleep_cts: qup-uart7-sleep-cts { + pins = "gpio28"; + function = "gpio"; + /* + * Configure a pull-down on CTS to match the pull of + * the Bluetooth module. + */ + bias-pull-down; + }; + + qup_uart7_sleep_rts: qup-uart7-sleep-rts { + pins = "gpio29"; + function = "gpio"; + /* + * Configure pull-down on RTS. As RTS is active low + * signal, pull it low to indicate the BT SoC that it + * can wakeup the system anytime from suspend state by + * pulling RX low (by sending wakeup bytes). + */ + bias-pull-down; + }; + + qup_uart7_sleep_tx: qup-uart7-sleep-tx { + pins = "gpio30"; + function = "gpio"; + /* + * Configure pull-up on TX when it isn't actively driven + * to prevent BT SoC from receiving garbage during sleep. + */ + bias-pull-up; + }; + + qup_uart7_sleep_rx: qup-uart7-sleep-rx {
arch/arm64/boot/dts/qcom/sc7280-idp.dts line 514
Quoted 10 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dts +10
-0+ pins = "gpio31"; + function = "gpio"; + /* + * Configure a pull-up on RX. This is needed to avoid + * garbage data when the TX pin of the Bluetooth module + * is floating which may cause spurious wakeups. + */ + bias-pull-up; + }; +};
How the patches of this series are split strikes me as a bit odd. Supposedly this patch adds the QUPv3 wrapper_0 DT nodes for the SC7280, however the above is the pin configuration for the Bluetooth UART of the SC7280 IDP board. I don't see a good reason why that should be part of this patch. It should be a separate change whose subject indicates that it configures the Bluetooth UART of the SC7280 IDP.
Without this conflation of SoC and board DT it would seem perfectly reasonable to squash this patch and '[4/4] arm64: dts: sc7280: Add QUPv3 wrapper_1 nodes' into a single one, they are essentially doing the same thing, I see no need to have different patches for the wrapper 0 and 1 nodes.
Re: [PATCH V4 2/4] arm64: dts: sc7280: Add QUPv3 wrapper_0 nodes
Quoted 128 lines · Matthias Kaehlcke
Quoted 127 lines · Rajesh Patil
From: Roja Rani Yarubandi <[email protected]>
Add QUPv3
wrapper_0DT nodes for SC7280 SoC.Signed-off-by Roja Rani Yarubandi <[email protected]>Signed-off-by Rajesh Patil <[email protected]>---
Changes in V4:
- As per Bjorn's comment, added QUPWrapper_0nodes other than debug-uart node
- Dropped interconnect votes forwrapper_0nodeChanges in V3:
- Broken the huge V2 patch into 3 smaller patches.
1. QSPI DT nodes
2. QUPwrapper_0DT nodes
3. QUPwrapper_1DT nodesChanges in V2:
- As per Doug's comments removed pinmux/pinconf subnodes.
- As per Doug's comments split of SPI, UART nodes has been done.
- Moved QSPI node beforeaps_smmuas per the order.arch/arm64/boot/dts/qcom/sc7280-idp.dts | 84 ++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 720 ++++++++++++++++++++++++++++++++ 2 files changed, 804 insertions(+)arch/arm64/boot/dts/qcom/sc7280-idp.dts +10
-0@@ -358,6 +358,16 @@ vdda18-supply = <&vreg_l1c_1p8>; }; +&uart7 { + status = "okay"; + + /delete-property/interrupts; + interrupts-extended = <&intc GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>, + <&tlmm 31 IRQ_TYPE_EDGE_FALLING>; + pinctrl-names = "default", "sleep"; + pinctrl-1 = <&qup_uart7_sleep_cts>, <&qup_uart7_sleep_rts>, <&qup_uart7_sleep_tx>, <&qup_uart7_sleep_rx>; +}; + /* PINCTRL - additions to nodes defined in sc7280.dtsi */
&qspi_cs0{arch/arm64/boot/dts/qcom/sc7280-idp.dts +72
-0@@ -428,3 +438,77 @@ bias-pull-up; }; }; +&qup_uart7_cts { + /* + * Configure a pull-down on CTS to match the pull of + * the Bluetooth module. + */ + bias-pull-down; +}; + +&qup_uart7_rts { + /* We'll drive RTS, so no pull */ + drive-strength = <2>; + bias-disable; +}; + +&qup_uart7_tx { + /* We'll drive TX, so no pull */ + drive-strength = <2>; + bias-disable; +}; + +&qup_uart7_rx { + /* + * Configure a pull-up on RX. This is needed to avoid + * garbage data when the TX pin of the Bluetooth module is + * in tri-state (module powered off or not driving the + * signal yet). + */ + bias-pull-up; +}; + +&tlmm { + qup_uart7_sleep_cts: qup-uart7-sleep-cts { + pins = "gpio28"; + function = "gpio"; + /* + * Configure a pull-down on CTS to match the pull of + * the Bluetooth module. + */ + bias-pull-down; + }; + + qup_uart7_sleep_rts: qup-uart7-sleep-rts { + pins = "gpio29"; + function = "gpio"; + /* + * Configure pull-down on RTS. As RTS is active low + * signal, pull it low to indicate the BT SoC that it + * can wakeup the system anytime from suspend state by + * pulling RX low (by sending wakeup bytes). + */ + bias-pull-down; + }; + + qup_uart7_sleep_tx: qup-uart7-sleep-tx { + pins = "gpio30"; + function = "gpio"; + /* + * Configure pull-up on TX when it isn't actively driven + * to prevent BT SoC from receiving garbage during sleep. + */ + bias-pull-up; + }; + qup_uart7_sleep_rx: qup-uart7-sleep-rx { + pins = "gpio31"; + function = "gpio"; + /* + * Configure a pull-up on RX. This is needed to avoid + * garbage data when the TX pin of the Bluetooth module + * is floating which may cause spurious wakeups. + */ + bias-pull-up; + };
Quoted 9 lines
Quoted 1 line
+};
How the patches of this series are split strikes me as a bit odd. Supposedly this patch adds the QUPv3
wrapper_0DT nodes for the SC7280, however the above is the pin configuration for the Bluetooth UART of the SC7280 IDP board. I don't see a good reason why that should be part of this patch. It should be a separate change whose subject indicates that it configures the Bluetooth UART of the SC7280 IDP.
Okay will split this up.
Quoted 4 lines
Without this conflation of SoC and board DT it would seem perfectly reasonable to squash this patch and '[4/4] arm64: dts: sc7280: Add QUPv3
wrapper_1nodes' into a single one, they are essentially doing the same thing, I see no need to have different patches for the wrapper 0 and 1 nodes.
Previously when QUP wrapper 0 and wrapper 1 nodes were added in single patch, we faced some git issues as the patch was huge. Hence we split it up.
https://partnerissuetracker.corp.google.com/issues/177045897#comment12
Re: [PATCH V4 2/4] arm64: dts: sc7280: Add QUPv3 wrapper_0 nodes
Quoted 154 lines · On Wed, Aug 11, 2021 at 05:43:48PM +0530, [email protected] wrote:
Quoted 133 lines · Matthias Kaehlcke
Quoted 119 lines · Rajesh Patil
From: Roja Rani Yarubandi <[email protected]>
Add QUPv3
wrapper_0DT nodes for SC7280 SoC.Signed-off-by Roja Rani Yarubandi <[email protected]>Signed-off-by Rajesh Patil <[email protected]>---
Changes in V4:
- As per Bjorn's comment, added QUPWrapper_0nodes other than debug-uart node
- Dropped interconnect votes forwrapper_0nodeChanges in V3:
- Broken the huge V2 patch into 3 smaller patches.
1. QSPI DT nodes
2. QUPwrapper_0DT nodes
3. QUPwrapper_1DT nodesChanges in V2:
- As per Doug's comments removed pinmux/pinconf subnodes.
- As per Doug's comments split of SPI, UART nodes has been done.
- Moved QSPI node beforeaps_smmuas per the order.arch/arm64/boot/dts/qcom/sc7280-idp.dts | 84 ++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 720 ++++++++++++++++++++++++++++++++ 2 files changed, 804 insertions(+)arch/arm64/boot/dts/qcom/sc7280-idp.dts +10
-0@@ -358,6 +358,16 @@ vdda18-supply = <&vreg_l1c_1p8>; }; +&uart7 { + status = "okay"; + + /delete-property/interrupts; + interrupts-extended = <&intc GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>, + <&tlmm 31 IRQ_TYPE_EDGE_FALLING>; + pinctrl-names = "default", "sleep"; + pinctrl-1 = <&qup_uart7_sleep_cts>, <&qup_uart7_sleep_rts>, <&qup_uart7_sleep_tx>, <&qup_uart7_sleep_rx>; +}; + /* PINCTRL - additions to nodes defined in sc7280.dtsi */
&qspi_cs0{arch/arm64/boot/dts/qcom/sc7280-idp.dts +63
-0@@ -428,3 +438,77 @@ bias-pull-up; }; }; +&qup_uart7_cts { + /* + * Configure a pull-down on CTS to match the pull of + * the Bluetooth module. + */ + bias-pull-down; +}; + +&qup_uart7_rts { + /* We'll drive RTS, so no pull */ + drive-strength = <2>; + bias-disable; +}; + +&qup_uart7_tx { + /* We'll drive TX, so no pull */ + drive-strength = <2>; + bias-disable; +}; + +&qup_uart7_rx { + /* + * Configure a pull-up on RX. This is needed to avoid + * garbage data when the TX pin of the Bluetooth module is + * in tri-state (module powered off or not driving the + * signal yet). + */ + bias-pull-up; +}; + +&tlmm { + qup_uart7_sleep_cts: qup-uart7-sleep-cts { + pins = "gpio28"; + function = "gpio"; + /* + * Configure a pull-down on CTS to match the pull of + * the Bluetooth module. + */ + bias-pull-down; + }; + + qup_uart7_sleep_rts: qup-uart7-sleep-rts { + pins = "gpio29"; + function = "gpio"; + /* + * Configure pull-down on RTS. As RTS is active low + * signal, pull it low to indicate the BT SoC that it + * can wakeup the system anytime from suspend state by + * pulling RX low (by sending wakeup bytes). + */ + bias-pull-down; + }; + + qup_uart7_sleep_tx: qup-uart7-sleep-tx { + pins = "gpio30"; + function = "gpio"; + /* + * Configure pull-up on TX when it isn't actively driven + * to prevent BT SoC from receiving garbage during sleep. + */ + bias-pull-up; + }; + qup_uart7_sleep_rx: qup-uart7-sleep-rx {arch/arm64/boot/dts/qcom/sc7280-idp.dts line 514
Quoted 10 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dts +10
-0+ pins = "gpio31"; + function = "gpio"; + /* + * Configure a pull-up on RX. This is needed to avoid + * garbage data when the TX pin of the Bluetooth module + * is floating which may cause spurious wakeups. + */ + bias-pull-up; + }; +};How the patches of this series are split strikes me as a bit odd. Supposedly
Quoted 9 lines
this patch adds the QUPv3
wrapper_0DT nodes for the SC7280, however the above is the pin configuration for the Bluetooth UART of the SC7280 IDP board.
I don't see a good reason why that should be part of this patch. It should be
a separate change whose subject indicates that it configures the Bluetooth UART
of the SC7280 IDP.Okay will split this up.
Quoted 7 lines
Without this conflation of SoC and board DT it would seem perfectly reasonable
to squash this patch and '[4/4] arm64: dts: sc7280: Add QUPv3wrapper_1nodes'
into a single one, they are essentially doing the same thing, I see no need to
have different patches for the wrapper 0 and 1 nodes.
Quoted 3 lines
Previously when QUP wrapper 0 and wrapper 1 nodes were added in single patch, we faced some git issues as the patch was huge. Hence we split it up.
https://partnerissuetracker.corp.google.com/issues/177045897#comment12
That bug tracker entry isn't public, this is what the comment says:
Quoted 8 lines
I suspect that diff is just having a hard time since your patch touches so much stuff. Presumably you could make it happier if you broke your patch into smaller changes. For instance:
One patch that adds the quad SPI. After all the quad SPI isn't really related to the other QUP stuff. One patch that fixes up the existing "uart5" and the QUP it's on to be how it's supposed to be. One patch that adds all the new stuff.
If I understand correctly the problem wasn't that the QUP wrappers are added in a single patch (which should be pretty straightforward to review), but that the previous patch(es) do too many things at once. Adding both QUP wrappers is conceptionally a single thing, the problem is intermingling that with adding board specific Bluetooth nodes, board flash nodes, pinconf for UART, etc
Re: [PATCH V4 2/4] arm64: dts: sc7280: Add QUPv3 wrapper_0 nodes
Quoted 170 lines · Matthias Kaehlcke
Quoted 154 lines · On Wed, Aug 11, 2021 at 05:43:48PM +0530, [email protected] wrote:
Quoted 133 lines · Matthias Kaehlcke
Quoted 119 lines · Rajesh Patil
From: Roja Rani Yarubandi <[email protected]>
Add QUPv3
wrapper_0DT nodes for SC7280 SoC.Signed-off-by Roja Rani Yarubandi <[email protected]>Signed-off-by Rajesh Patil <[email protected]>---
Changes in V4:
- As per Bjorn's comment, added QUPWrapper_0nodes other than debug-uart node
- Dropped interconnect votes forwrapper_0nodeChanges in V3:
- Broken the huge V2 patch into 3 smaller patches.
1. QSPI DT nodes
2. QUPwrapper_0DT nodes
3. QUPwrapper_1DT nodesChanges in V2:
- As per Doug's comments removed pinmux/pinconf subnodes.
- As per Doug's comments split of SPI, UART nodes has been done.
- Moved QSPI node beforeaps_smmuas per the order.arch/arm64/boot/dts/qcom/sc7280-idp.dts | 84 ++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 720 ++++++++++++++++++++++++++++++++ 2 files changed, 804 insertions(+)arch/arm64/boot/dts/qcom/sc7280-idp.dts +10
-0@@ -358,6 +358,16 @@ vdda18-supply = <&vreg_l1c_1p8>; }; +&uart7 { + status = "okay"; + + /delete-property/interrupts; + interrupts-extended = <&intc GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>, + <&tlmm 31 IRQ_TYPE_EDGE_FALLING>; + pinctrl-names = "default", "sleep"; + pinctrl-1 = <&qup_uart7_sleep_cts>, <&qup_uart7_sleep_rts>, <&qup_uart7_sleep_tx>, <&qup_uart7_sleep_rx>; +}; + /* PINCTRL - additions to nodes defined in sc7280.dtsi */
&qspi_cs0{arch/arm64/boot/dts/qcom/sc7280-idp.dts +63
-0@@ -428,3 +438,77 @@ bias-pull-up; }; }; +&qup_uart7_cts { + /* + * Configure a pull-down on CTS to match the pull of + * the Bluetooth module. + */ + bias-pull-down; +}; + +&qup_uart7_rts { + /* We'll drive RTS, so no pull */ + drive-strength = <2>; + bias-disable; +}; + +&qup_uart7_tx { + /* We'll drive TX, so no pull */ + drive-strength = <2>; + bias-disable; +}; + +&qup_uart7_rx { + /* + * Configure a pull-up on RX. This is needed to avoid + * garbage data when the TX pin of the Bluetooth module is + * in tri-state (module powered off or not driving the + * signal yet). + */ + bias-pull-up; +}; + +&tlmm { + qup_uart7_sleep_cts: qup-uart7-sleep-cts { + pins = "gpio28"; + function = "gpio"; + /* + * Configure a pull-down on CTS to match the pull of + * the Bluetooth module. + */ + bias-pull-down; + }; + + qup_uart7_sleep_rts: qup-uart7-sleep-rts { + pins = "gpio29"; + function = "gpio"; + /* + * Configure pull-down on RTS. As RTS is active low + * signal, pull it low to indicate the BT SoC that it + * can wakeup the system anytime from suspend state by + * pulling RX low (by sending wakeup bytes). + */ + bias-pull-down; + }; + + qup_uart7_sleep_tx: qup-uart7-sleep-tx { + pins = "gpio30"; + function = "gpio"; + /* + * Configure pull-up on TX when it isn't actively driven + * to prevent BT SoC from receiving garbage during sleep. + */ + bias-pull-up; + }; + qup_uart7_sleep_rx: qup-uart7-sleep-rx {arch/arm64/boot/dts/qcom/sc7280-idp.dts line 514
Quoted 10 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dts +10
-0+ pins = "gpio31"; + function = "gpio"; + /* + * Configure a pull-up on RX. This is needed to avoid + * garbage data when the TX pin of the Bluetooth module + * is floating which may cause spurious wakeups. + */ + bias-pull-up; + }; +};How the patches of this series are split strikes me as a bit odd. Supposedly
Quoted 9 lines
this patch adds the QUPv3
wrapper_0DT nodes for the SC7280, however the above is the pin configuration for the Bluetooth UART of the SC7280 IDP board.
I don't see a good reason why that should be part of this patch. It should be
a separate change whose subject indicates that it configures the Bluetooth UART
of the SC7280 IDP.Okay will split this up.
Quoted 7 lines
Without this conflation of SoC and board DT it would seem perfectly reasonable
to squash this patch and '[4/4] arm64: dts: sc7280: Add QUPv3wrapper_1nodes'
into a single one, they are essentially doing the same thing, I see no need to
have different patches for the wrapper 0 and 1 nodes.Quoted 3 lines
Previously when QUP wrapper 0 and wrapper 1 nodes were added in single patch, we faced some git issues as the patch was huge. Hence we split it up.
https://partnerissuetracker.corp.google.com/issues/177045897#comment12That bug tracker entry isn't public, this is what the comment says:
Quoted 8 lines
I suspect that diff is just having a hard time since your patch touches so much stuff. Presumably you could make it happier if you broke your patch into smaller changes. For instance:
One patch that adds the quad SPI. After all the quad SPI isn't really related to the other QUP stuff. One patch that fixes up the existing "uart5" and the QUP it's on to be how it's supposed to be. One patch that adds all the new stuff.
Quoted 6 lines
If I understand correctly the problem wasn't that the QUP wrappers are added in a single patch (which should be pretty straightforward to review), but that the previous patch(es) do too many things at once. Adding both QUP wrappers is conceptionally a single thing, the problem is intermingling that with adding board specific Bluetooth nodes, board flash nodes, pinconf for UART, etc
Even after splitting the patches as suggested(i.e., additions and modifications separately), if I add qup wrapper0 and wrapper1 nodes in single patch the git diff is getting messed up. pasted the diff for reference
+ qup_spi14_cs_gpio: qup-spi14-cs_gpio {
+ pins = "gpio59";
+ function = "gpio";
+ };
- assigned-clocks = <&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>,
- <&gcc GCC_USB30_PRIM_MASTER_CLK>;
- assigned-clock-rates = <19200000>, <200000000>;
+ qup_spi15_data_clk: qup-spi15-data-clk {
+ pins = "gpio60", "gpio61", "gpio62";
+ function = "qup17";
+ };
- interrupts-extended = <&intc GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
- <&pdc 14 IRQ_TYPE_EDGE_BOTH>, - <&pdc 15 IRQ_TYPE_EDGE_BOTH>, - <&pdc 17 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "hs_phy_irq", "dp_hs_phy_irq",
- "dm_hs_phy_irq", "ss_phy_irq";
+ qup_spi15_cs: qup-spi15-cs {
+ pins = "gpio63";
+ function = "qup17";
+ };
- power-domains = <&gcc GCC_USB30_PRIM_GDSC>;
+ qup_spi15_cs_gpio: qup-spi15-cs_gpio {
+ pins = "gpio63";
+ function = "gpio";
+ };
Re: [PATCH V4 2/4] arm64: dts: sc7280: Add QUPv3 wrapper_0 nodes
Quoted 210 lines · On Thu, Aug 12, 2021 at 12:43:11PM +0530, [email protected] wrote:
Quoted 173 lines · Matthias Kaehlcke
Quoted 154 lines · On Wed, Aug 11, 2021 at 05:43:48PM +0530, [email protected] wrote:
Quoted 133 lines · Matthias Kaehlcke
Quoted 119 lines · Rajesh Patil
From: Roja Rani Yarubandi <[email protected]>
Add QUPv3
wrapper_0DT nodes for SC7280 SoC.Signed-off-by Roja Rani Yarubandi <[email protected]>Signed-off-by Rajesh Patil <[email protected]>---
Changes in V4:
- As per Bjorn's comment, added QUPWrapper_0nodes other than debug-uart node
- Dropped interconnect votes forwrapper_0nodeChanges in V3:
- Broken the huge V2 patch into 3 smaller patches.
1. QSPI DT nodes
2. QUPwrapper_0DT nodes
3. QUPwrapper_1DT nodesChanges in V2:
- As per Doug's comments removed pinmux/pinconf subnodes.
- As per Doug's comments split of SPI, UART nodes has been done.
- Moved QSPI node beforeaps_smmuas per the order.arch/arm64/boot/dts/qcom/sc7280-idp.dts | 84 ++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 720 ++++++++++++++++++++++++++++++++ 2 files changed, 804 insertions(+)arch/arm64/boot/dts/qcom/sc7280-idp.dts +10
-0@@ -358,6 +358,16 @@ vdda18-supply = <&vreg_l1c_1p8>; }; +&uart7 { + status = "okay"; + + /delete-property/interrupts; + interrupts-extended = <&intc GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>, + <&tlmm 31 IRQ_TYPE_EDGE_FALLING>; + pinctrl-names = "default", "sleep"; + pinctrl-1 = <&qup_uart7_sleep_cts>, <&qup_uart7_sleep_rts>, <&qup_uart7_sleep_tx>, <&qup_uart7_sleep_rx>; +}; + /* PINCTRL - additions to nodes defined in sc7280.dtsi */
&qspi_cs0{arch/arm64/boot/dts/qcom/sc7280-idp.dts +63
-0@@ -428,3 +438,77 @@ bias-pull-up; }; }; +&qup_uart7_cts { + /* + * Configure a pull-down on CTS to match the pull of + * the Bluetooth module. + */ + bias-pull-down; +}; + +&qup_uart7_rts { + /* We'll drive RTS, so no pull */ + drive-strength = <2>; + bias-disable; +}; + +&qup_uart7_tx { + /* We'll drive TX, so no pull */ + drive-strength = <2>; + bias-disable; +}; + +&qup_uart7_rx { + /* + * Configure a pull-up on RX. This is needed to avoid + * garbage data when the TX pin of the Bluetooth module is + * in tri-state (module powered off or not driving the + * signal yet). + */ + bias-pull-up; +}; + +&tlmm { + qup_uart7_sleep_cts: qup-uart7-sleep-cts { + pins = "gpio28"; + function = "gpio"; + /* + * Configure a pull-down on CTS to match the pull of + * the Bluetooth module. + */ + bias-pull-down; + }; + + qup_uart7_sleep_rts: qup-uart7-sleep-rts { + pins = "gpio29"; + function = "gpio"; + /* + * Configure pull-down on RTS. As RTS is active low + * signal, pull it low to indicate the BT SoC that it + * can wakeup the system anytime from suspend state by + * pulling RX low (by sending wakeup bytes). + */ + bias-pull-down; + }; + + qup_uart7_sleep_tx: qup-uart7-sleep-tx { + pins = "gpio30"; + function = "gpio"; + /* + * Configure pull-up on TX when it isn't actively driven + * to prevent BT SoC from receiving garbage during sleep. + */ + bias-pull-up; + }; + qup_uart7_sleep_rx: qup-uart7-sleep-rx {arch/arm64/boot/dts/qcom/sc7280-idp.dts line 514
Quoted 10 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dts +10
-0+ pins = "gpio31"; + function = "gpio"; + /* + * Configure a pull-up on RX. This is needed to avoid + * garbage data when the TX pin of the Bluetooth module + * is floating which may cause spurious wakeups. + */ + bias-pull-up; + }; +};How the patches of this series are split strikes me as a bit odd. Supposedly
Quoted 9 lines
this patch adds the QUPv3
wrapper_0DT nodes for the SC7280, however the above is the pin configuration for the Bluetooth UART of the SC7280 IDP board.
I don't see a good reason why that should be part of this patch. It should be
a separate change whose subject indicates that it configures the Bluetooth UART
of the SC7280 IDP.Okay will split this up.
Quoted 7 lines
Without this conflation of SoC and board DT it would seem perfectly reasonable
to squash this patch and '[4/4] arm64: dts: sc7280: Add QUPv3wrapper_1nodes'
into a single one, they are essentially doing the same thing, I see no need to
have different patches for the wrapper 0 and 1 nodes.Quoted 4 lines
Previously when QUP wrapper 0 and wrapper 1 nodes were added in single patch, we faced some git issues as the patch was huge. Hence we split it up.
https://partnerissuetracker.corp.google.com/issues/177045897#comment12That bug tracker entry isn't public, this is what the comment says:
Quoted 10 lines
I suspect that diff is just having a hard time since your patch touches so
much stuff. Presumably you could make it happier if you broke your patch
into smaller changes. For instance:One patch that adds the quad SPI. After all the quad SPI isn't really related to the other QUP stuff. One patch that fixes up the existing "uart5" and the QUP it's on to be how it's supposed to be. One patch that adds all the new stuff.
Quoted 6 lines
If I understand correctly the problem wasn't that the QUP wrappers are added in a single patch (which should be pretty straightforward to review), but that the previous patch(es) do too many things at once. Adding both QUP wrappers is conceptionally a single thing, the problem is intermingling that with adding board specific Bluetooth nodes, board flash nodes, pinconf for UART, etc
Even after splitting the patches as
suggested(i.e., additions and modifications separately), if I add qup wrapper0 and wrapper1 nodes in single patch the git diff is getting messed up. pasted the diff for reference(quoted diff) +4
-1+ qup_spi14_cs_gpio: qup-spi14-cs_gpio { + pins = "gpio59"; + function = "gpio"; + }; - assigned-clocks = <&gcc
GCC_USB30_PRIM_MOCK_UTMI_CLK>,(quoted diff) +4
-3- <&gcc GCC_USB30_PRIM_MASTER_CLK>; - assigned-clock-rates = <19200000>, <200000000>; + qup_spi15_data_clk: qup-spi15-data-clk { + pins = "gpio60", "gpio61", "gpio62"; + function = "qup17"; + }; - interrupts-extended = <&intc GIC_SPI 131
IRQ_TYPE_LEVEL_HIGH>,(quoted diff) +1
-5- <&pdc 14 IRQ_TYPE_EDGE_BOTH>, - <&pdc 15 IRQ_TYPE_EDGE_BOTH>, - <&pdc 17 IRQ_TYPE_LEVEL_HIGH>; - interrupt-names = "hs_phy_irq", "dp_hs_phy_irq", - "dm_hs_phy_irq", "ss_phy_irq"; + qup_spi15_cs: qup-spi15-cs {
Review
Quoted 10 lines
(quoted diff) +7
-1+ pins = "gpio63"; + function = "qup17"; + }; - power-domains = <&gcc GCC_USB30_PRIM_GDSC>; + qup_spi15_cs_gpio: qup-spi15-cs_gpio { + pins = "gpio63"; + function = "gpio"; + };
If the diffs actually end up more messy with a single patch in this case then there is no point for it. Thanks for trying.
Re: [PATCH V4 2/4] arm64: dts: sc7280: Add QUPv3 wrapper_0 nodes
In reply to Rajesh PatilQuoting Rajesh Patil (2021-07-26 06:40:45)
arch/arm64/boot/dts/qcom/sc7280.dtsi line 523
Quoted 9 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi +1
-0@@ -520,6 +520,25 @@ }; + qup_opp_table: qup-opp-table {
Surely this can live underneath a qup node parallel to the i2c and spi devices?
Quoted 21 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi +18
-0+ compatible = "operating-points-v2"; + + opp-75000000 { + opp-hz = /bits/ 64 <75000000>; + required-opps = <&rpmhpd_opp_low_svs>; + }; + + opp-100000000 { + opp-hz = /bits/ 64 <100000000>; + required-opps = <&rpmhpd_opp_svs>; + }; + + opp-128000000 { + opp-hz = /bits/ 64 <128000000>; + required-opps = <&rpmhpd_opp_nom>; + }; + }; + qupv3_id_0: geniqup@9c0000 { compatible = "qcom,geni-se-qup"; reg = <0 0x009c0000 0 0x2000>;
Re: [PATCH V4 2/4] arm64: dts: sc7280: Add QUPv3 wrapper_0 nodes
Quoted 7 lines · Stephen Boyd
Quoting Rajesh Patil (2021-07-26 06:40:45)
Quoted 6 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi +0
-0@@ -520,6 +520,25 @@
Quoted 7 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi line 523
Quoted 3 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi +1
-0}; + qup_opp_table: qup-opp-table {Surely this can live underneath a qup node parallel to the i2c and spi devices?
But this is common to both qup0 and qup1 right?
Quoted 21 lines
Quoted 21 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi +18
-0+ compatible = "operating-points-v2"; + + opp-75000000 { + opp-hz = /bits/ 64 <75000000>; + required-opps = <&rpmhpd_opp_low_svs>; + }; + + opp-100000000 { + opp-hz = /bits/ 64 <100000000>; + required-opps = <&rpmhpd_opp_svs>; + }; + + opp-128000000 { + opp-hz = /bits/ 64 <128000000>; + required-opps = <&rpmhpd_opp_nom>; + }; + }; + qupv3_id_0: geniqup@9c0000 { compatible = "qcom,geni-se-qup"; reg = <0 0x009c0000 0 0x2000>;
PATCH v4 3/4 arm64: dts: sc7280: Update QUPv3 Debug UART DT node
In reply to Rajesh PatilFrom: Roja Rani Yarubandi <[email protected]>
Update QUPv3 Debug UART DT node with the interconnect names and functions for SC7280 SoC.
Split the Debug UART pin control functions.
---
Changes in V4:
- As per Bjorn's comment, posting this debug-uart node update as seperate patch
arch/arm64/boot/dts/qcom/sc7280-idp.dts | 18 +++++++----------- arch/arm64/boot/dts/qcom/sc7280.dtsi | 28 ++++++++++++++++++++++++---- 2 files changed, 31 insertions(+), 15 deletions(-)
arch/arm64/boot/dts/qcom/sc7280-idp.dts +7 -11
@@ -383,18 +383,14 @@ bias-pull-up; }; -&qup_uart5_default { - tx { - pins = "gpio46"; - drive-strength = <2>; - bias-disable; - }; +&qup_uart5_tx { + drive-strength = <2>; + bias-disable; +}; - rx { - pins = "gpio47"; - drive-strength = <2>; - bias-pull-up; - }; +&qup_uart5_rx { + drive-strength = <2>; + bias-pull-up; }; &sdc1_on {
arch/arm64/boot/dts/qcom/sc7280.dtsi +24 -4
@@ -853,8 +853,13 @@ clock-names = "se"; clocks = <&gcc GCC_QUPV3_WRAP0_S5_CLK>; pinctrl-names = "default"; - pinctrl-0 = <&qup_uart5_default>; + pinctrl-0 = <&qup_uart5_cts>, <&qup_uart5_rts>, <&qup_uart5_tx>, <&qup_uart5_rx>; interrupts = <GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH>; + power-domains = <&rpmhpd SC7280_CX>; + operating-points-v2 = <&qup_opp_table>; + interconnects = <&clk_virt MASTER_QUP_CORE_0 0 &clk_virt SLAVE_QUP_CORE_0 0>, + <&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_0 0>; + interconnect-names = "qup-core", "qup-config"; status = "disabled"; };
@@ -2234,9 +2239,24 @@ function = "qup04"; }; - qup_uart5_default: qup-uart5-default { - pins = "gpio46", "gpio47"; - function = "qup13"; + qup_uart5_cts: qup-uart5-cts { + pins = "gpio20"; + function = "qup05"; + }; + + qup_uart5_rts: qup-uart5-rts { + pins = "gpio21"; + function = "qup05"; + }; + + qup_uart5_tx: qup-uart5-tx { + pins = "gpio22"; + function = "qup05"; + }; + + qup_uart5_rx: qup-uart5-rx { + pins = "gpio23"; + function = "qup05"; }; qup_uart6_cts: qup-uart6-cts {
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH V4 3/4] arm64: dts: sc7280: Update QUPv3 Debug UART DT node
Quoted 54 lines · Rajesh Patil
From: Roja Rani Yarubandi <[email protected]>
Update QUPv3 Debug UART DT node with the interconnect names and functions for SC7280 SoC.
Split the Debug UART pin control functions.
Signed-off-by Roja Rani Yarubandi <[email protected]>Signed-off-by Rajesh Patil <[email protected]>---
Changes in V4:
- As per Bjorn's comment, posting this debug-uart node update as seperate patcharch/arm64/boot/dts/qcom/sc7280-idp.dts | 18 +++++++----------- arch/arm64/boot/dts/qcom/sc7280.dtsi | 28 ++++++++++++++++++++++++---- 2 files changed, 31 insertions(+), 15 deletions(-)arch/arm64/boot/dts/qcom/sc7280-idp.dts +7
-11@@ -383,18 +383,14 @@ bias-pull-up; }; -&qup_uart5_default { - tx { - pins = "gpio46"; - drive-strength = <2>; - bias-disable; - }; +&qup_uart5_tx { + drive-strength = <2>; + bias-disable; +}; - rx { - pins = "gpio47"; - drive-strength = <2>; - bias-pull-up; - }; +&qup_uart5_rx { + drive-strength = <2>; + bias-pull-up; }; &sdc1_on {arch/arm64/boot/dts/qcom/sc7280.dtsi +0
-0@@ -853,8 +853,13 @@ clock-names = "se";
arch/arm64/boot/dts/qcom/sc7280.dtsi line 862
Quoted 10 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi +6
-1clocks = <&gcc GCC_QUPV3_WRAP0_S5_CLK>; pinctrl-names = "default"; - pinctrl-0 = <&qup_uart5_default>; + pinctrl-0 = <&qup_uart5_cts>, <&qup_uart5_rts>, <&qup_uart5_tx>, <&qup_uart5_rx>; interrupts = <GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH>; + power-domains = <&rpmhpd SC7280_CX>; + operating-points-v2 = <&qup_opp_table>; + interconnects = <&clk_virt MASTER_QUP_CORE_0 0 &clk_virt SLAVE_QUP_CORE_0 0>, + <&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_0 0>; + interconnect-names = "qup-core", "qup-config";
Most of the above should be added by patch '[2/4] arm64: dts: sc7280: Add QUPv3 wrapper_0 nodes'.
I have to say I dislike that the SoC DT file dictates which UART to use for the serial console. Technically it could be any of them, right? uart5 is used because that's what the IDP does, and the rest of the world is expected to follow. Why not configure uart5 as "qcom,geni-uart" by default and overwrite the compatible string and pinctrl in the board file? You could even add 'qup-uartN-all' (or similar) pinconfigs to sc7280.dtsi, which would make the changes in the board file trivial.
Re: [PATCH V4 3/4] arm64: dts: sc7280: Update QUPv3 Debug UART DT node
Quoted 64 lines · Matthias Kaehlcke
Quoted 63 lines · Rajesh Patil
From: Roja Rani Yarubandi <[email protected]>
Update QUPv3 Debug UART DT node with the interconnect names and functions for SC7280 SoC.
Split the Debug UART pin control functions.
Signed-off-by Roja Rani Yarubandi <[email protected]>Signed-off-by Rajesh Patil <[email protected]>---
Changes in V4:
- As per Bjorn's comment, posting this debug-uart node update as seperate patcharch/arm64/boot/dts/qcom/sc7280-idp.dts | 18 +++++++----------- arch/arm64/boot/dts/qcom/sc7280.dtsi | 28 ++++++++++++++++++++++++---- 2 files changed, 31 insertions(+), 15 deletions(-)arch/arm64/boot/dts/qcom/sc7280-idp.dts +7
-11@@ -383,18 +383,14 @@ bias-pull-up; }; -&qup_uart5_default { - tx { - pins = "gpio46"; - drive-strength = <2>; - bias-disable; - }; +&qup_uart5_tx { + drive-strength = <2>; + bias-disable; +}; - rx { - pins = "gpio47"; - drive-strength = <2>; - bias-pull-up; - }; +&qup_uart5_rx { + drive-strength = <2>; + bias-pull-up; }; &sdc1_on {arch/arm64/boot/dts/qcom/sc7280.dtsi +3
-1@@ -853,8 +853,13 @@ clock-names = "se"; clocks = <&gcc GCC_QUPV3_WRAP0_S5_CLK>; pinctrl-names = "default"; - pinctrl-0 = <&qup_uart5_default>; + pinctrl-0 = <&qup_uart5_cts>, <&qup_uart5_rts>, <&qup_uart5_tx>, <&qup_uart5_rx>; interrupts = <GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH>; + power-domains = <&rpmhpd SC7280_CX>; + operating-points-v2 = <&qup_opp_table>;
Quoted 8 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi line 862
Quoted 4 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi +3
-0+ interconnects = <&clk_virt MASTER_QUP_CORE_0 0 &clk_virt SLAVE_QUP_CORE_0 0>, + <&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_0 0>; + interconnect-names = "qup-core", "qup-config";Most of the above should be added by patch '[2/4] arm64: dts: sc7280: Add QUPv3
wrapper_0nodes'.
Based on the comments on v3 [1], I have added this as a separate patch
[1] https://lore.kernel.org/patchwork/patch/1441257/
Quoted 8 lines
I have to say I dislike that the SoC DT file dictates which UART to use for the serial console. Technically it could be any of them, right? uart5 is used because that's what the IDP does, and the rest of the world is expected to follow. Why not configure uart5 as "qcom,geni-uart" by default and overwrite the compatible string and pinctrl in the board file? You could even add 'qup-uartN-all' (or similar) pinconfigs to
sc7280.dtsi, which would make the changes in the board file trivial.
Okay, will make the compatible as "qcom,geni-uart" in SoC dt and later modify it in idp dts as "qcom,geni-debug-uart".
Thanks,
Rajesh
PATCH v4 4/4 arm64: dts: sc7280: Add QUPv3 wrapper_1 nodes
In reply to Rajesh PatilFrom: Roja Rani Yarubandi <[email protected]>
Add QUPv3 wrapper_1 DT nodes for SC7280 SoC.
---
Changes in V4:
- Dropped interconnect votes for wrapper_1 node
- Corrected QUP Wrapper_1 SE node's pin control functions like below QUP Wrapper_0: SE0-SE7 uses qup00 - qup07 pin-cntrl functions. QUP Wrapper_1: SE0-SE7 uses qup10 - qup17 pin-cntrl functions.
Changes in V3:
- Broken the huge V2 patch into 3 smaller patches.
1. QSPI DT nodes
2. QUP wrapper_0 DT nodes
3. QUP wrapper_1 DT nodes
Changes in V2:
- As per Doug's comments removed pinmux/pinconf subnodes.
- As per Doug's comments split of SPI, UART nodes has been done.
- Moved QSPI node before aps_smmu as per the order.
arch/arm64/boot/dts/qcom/sc7280-idp.dts | 4 + arch/arm64/boot/dts/qcom/sc7280.dtsi | 749 ++++++++++++++++++++++++++++++++ 2 files changed, 753 insertions(+)
arch/arm64/boot/dts/qcom/sc7280-idp.dts +4 -0
@@ -315,6 +315,10 @@ cd-gpios = <&tlmm 91 GPIO_ACTIVE_LOW>; }; +&qupv3_id_1 { + status = "okay"; +}; + &uart5 { status = "okay"; };
arch/arm64/boot/dts/qcom/sc7280.dtsi +749 -0
773 lines not on this page · raw message
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Quoting Rajesh Patil (2021-07-26 06:40:43)
Quoted 5 lines
Roja Rani Yarubandi (4):
arm64: dts: sc7280: Add QSPI node
arm64: dts: sc7280: Add QUPv3wrapper_0nodes
arm64: dts: sc7280: Update QUPv3 Debug UART DT node
arm64: dts: sc7280: Add QUPv3wrapper_1nodes
Please include a changelog in the cover letter and Cc me on the subsequent rounds of this series. Thanks.
Quoted 5 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dts | 133 +- arch/arm64/boot/dts/qcom/sc7280.dtsi | 3169 +++++++++++++++++++++++-------- 2 files changed, 2482 insertions(+), 820 deletions(-)
Quoted 10 lines · Stephen Boyd
Quoting Rajesh Patil (2021-07-26 06:40:43)
Quoted 5 lines
Roja Rani Yarubandi (4):
arm64: dts: sc7280: Add QSPI node
arm64: dts: sc7280: Add QUPv3wrapper_0nodes
arm64: dts: sc7280: Update QUPv3 Debug UART DT node
arm64: dts: sc7280: Add QUPv3wrapper_1nodesPlease include a changelog in the cover letter and Cc me on the subsequent rounds of this series. Thanks.
Okay.
Quoted 6 lines
Quoted 6 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dts | 133 +- arch/arm64/boot/dts/qcom/sc7280.dtsi | 3169 +++++++++++++++++++++++-------- 2 files changed, 2482 insertions(+), 820 deletions(-)