Skip to contentSkip to conversation

arm64: dts: sc7280: Add qspi, qupv3_0 and qupv3_1 nodes

PATCH Superseded Roja Rani Yarubandi 3 participants 4 messages Mar 11, 2021 – Apr 22, 2021

Subsystems (4)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 arm64: dts: sc7280: Add qspi, qupv3_0 and qupv3_1 nodes

Add QSPI and QUP SE instances configuration for sc7280.

---
This patch depends on base dtsi and interconnect changes
[1] https://lore.kernel.org/patchwork/project/lkml/list/?series=487403
[2] https://lore.kernel.org/patchwork/project/lkml/list/?series=488429

 arch/arm64/boot/dts/qcom/sc7280-idp.dts |  140 ++-
 arch/arm64/boot/dts/qcom/sc7280.dtsi    | 1296 ++++++++++++++++++++++-
 2 files changed, 1430 insertions(+), 6 deletions(-)
arch/arm64/boot/dts/qcom/sc7280-idp.dts +136 -4
index 950ecb25f281..e152d5a51c6b 100644
@@ -15,6 +15,7 @@ / {
 
 	aliases {
 		serial0 = &uart5;
+		hsuart0 = &uart7;
 	};
 
 	chosen {
@@ -22,26 +23,157 @@ chosen {
 	};
 };
 
+&qspi {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>;
+
+	flash@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+
+		/* TODO: Increase frequency after testing */
+		spi-max-frequency = <25000000>;
+		spi-tx-bus-width = <2>;
+		spi-rx-bus-width = <2>;
+	};
+};
+
 &qupv3_id_0 {
 	status = "okay";
 };
 
+&qupv3_id_1 {
+	status = "okay";
+};
+
 &uart5 {
 	status = "okay";
 };
 
+&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>;
+};
+
 /* PINCTRL - additions to nodes defined in sc7280.dtsi */
 
+&qspi_cs0 {
+	pinconf {
+		pins = "gpio15";
+		bias-disable;
+	};
+};
+
+&qspi_clk {
+	pinconf {
+		pins = "gpio14";
+		bias-disable;
+	};
+};
+
+&qspi_data01 {
+	pinconf {
+		pins = "gpio12", "gpio13";
+
+		/* High-Z when no transfers; nice to park the lines */
+		bias-pull-up;
+	};
+};
+
 &qup_uart5_default {
-	tx {
-		pins = "gpio46";
+	pinconf-tx {
+		pins = "gpio22";
 		drive-strength = <2>;
 		bias-disable;
 	};
 
-	rx {
-		pins = "gpio47";
+	pinconf-rx {
+		pins = "gpio23";
 		drive-strength = <2>;
 		bias-pull-up;
 	};
 };
+
+&qup_uart7_default {
+	pinconf-cts {
+		/*
+		 * Configure a pull-down on CTS to match the pull of
+		 * the Bluetooth module.
+		 */
+		pins = "gpio28";
+		bias-pull-down;
+	};
+
+	pinconf-rts-tx {
+		/* We'll drive RTS and TX, so no pull */
+		pins = "gpio29", "gpio30";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	pinconf-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).
+		 */
+		pins = "gpio31";
+		bias-pull-up;
+	};
+};
+
+&tlmm {
+	qup_uart7_sleep: qup-uart7-sleep {
+		pinmux {
+			pins = "gpio28", "gpio29",
+			       "gpio30", "gpio31";
+			function = "gpio";
+		};
+
+		pinconf-cts {
+			/*
+			 * Configure a pull-down on CTS to match the pull of
+			 * the Bluetooth module.
+			 */
+			pins = "gpio28";
+			bias-pull-down;
+		};
+
+		pinconf-rts {
+			/*
+			 * 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).
+			 */
+			 pins = "gpio29";
+			 bias-pull-down;
+		};
+
+		pinconf-tx {
+			/*
+			 * Configure pull-up on TX when it isn't actively driven
+			 * to prevent BT SoC from receiving garbage during sleep.
+			 */
+			pins = "gpio30";
+			bias-pull-up;
+		};
+
+		pinconf-rx {
+			/*
+			 * 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.
+			 */
+			pins = "gpio31";
+			bias-pull-up;
+		};
+	};
+};
arch/arm64/boot/dts/qcom/sc7280.dtsi +1294 -2

1,329 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
#1 lore raw

Hi,

arch/arm64/boot/dts/qcom/sc7280-idp.dts chosen { · line 70

Quoted 6 lines · Roja Rani Yarubandi

+&qspi_cs0 {

arch/arm64/boot/dts/qcom/sc7280-idp.dts +4 -0
+       pinconf {
+               pins = "gpio15";
+               bias-disable;
+       };

The "pinconf" / "pinmux" subnode shouldn't be used for new SoCs. See:

http://lore.kernel.org/r/CAD=FV=UY_AFRrAY0tef5jP698LEng6oN652LcX3B4nG=aWh0gA@mail.gmail.com

...same feedback for this whole patch.

arch/arm64/boot/dts/qcom/sc7280.dtsi tlmm: pinctrl@f100000 { · line 1363

Quoted 7 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi +7 -0
+                       qup_spi0_default: qup-spi0-default {
+                               pinmux {
+                                       pins = "gpio0", "gpio1",
+                                              "gpio2", "gpio3";
+                                       function = "qup00";
+                               };
+                       };

Please split these SPI nodes as per the thread above, like:

tlmm: pinctrl@... {

  qup_spi0_data_clk: qup-spi0-data-clk {
    pins = "gpio0", "gpio1", "gpio2";
    function = "qup0";

};

  qup_spi0_cs: qup-spi0-cs {
    pins = "gpio3";
    function = "qup0";
  };
  qup_spi0_cs_gpio: qup-spi0-cs-gpio {
    pins = "gpio3";
    function = "gpio";

};
};

arch/arm64/boot/dts/qcom/sc7280.dtsi tlmm: pinctrl@f100000 { · line 1491

Quoted 7 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi +7 -0
+                       qup_uart0_default: qup-uart0-default {
+                               pinmux {
+                                       pins = "gpio0", "gpio1",
+                                              "gpio2", "gpio3";
+                                       function = "qup00";
+                               };
+                       };

I suspect things would actually be cleaner if we broke the uart lines up since the boards tend to have to adjust pulls differently for each line. With the new "no pinconf / pinmux" world it's pretty clean. It's obviously up to Bjorn, but if it were me I'd request this in the SoC file:

qup_uart0_cts: qup-uart0-cts {
pins = "...";
function = "qup00";
};

qup_uart0_rts: qup-uart0-rts {
pins = "...";
function = "qup00";
};

qup_uart0_rx: qup-uart0-rx {
pins = "...";
function = "qup00";
};

qup_uart0_tx: qup-uart0-tx {
pins = "...";
function = "qup00";
};

...and now when the board file wants to adjust the pulls they can just reference each one:

/*
* Comments about why the UART0 pulls make sense.
* Blah blah blah.
*/

&qup_uart0_cts {
bias-pull-down;
};

&qup_uart0_rts {
drive-strength = <2>;
bias-disable;
};

&qup_uart0_rx {
bias-pull-up;
};

&qup_uart0_tx {
drive-strength = <2>;
bias-disable;
};

Quoted 1 line

+ qspi: spi@88dc000 {

I believe the qspi node is sorted incorrectly. When I apply this to the top of the Qualcomm tree it shows up after the "apps_smmu:
iommu@15000000" node. However:

0x088dc000 < 0x15000000

...which means it should be _before_.

-Doug

#2 lore raw
Quoted 53 lines · Doug Anderson

Hi,

arch/arm64/boot/dts/qcom/sc7280-idp.dts chosen { · line 70

Quoted 6 lines · Roja Rani Yarubandi

+&qspi_cs0 {

arch/arm64/boot/dts/qcom/sc7280-idp.dts +4 -0
+       pinconf {
+               pins = "gpio15";
+               bias-disable;
+       };

The "pinconf" / "pinmux" subnode shouldn't be used for new SoCs. See:

http://lore.kernel.org/r/CAD=FV=UY_AFRrAY0tef5jP698LEng6oN652LcX3B4nG=aWh0gA@mail.gmail.com

...same feedback for this whole patch.

arch/arm64/boot/dts/qcom/sc7280.dtsi tlmm: pinctrl@f100000 { · line 1363

Quoted 7 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi +7 -0
+                       qup_spi0_default: qup-spi0-default {
+                               pinmux {
+                                       pins = "gpio0", "gpio1",
+                                              "gpio2", "gpio3";
+                                       function = "qup00";
+                               };
+                       };

Please split these SPI nodes as per the thread above, like:

tlmm: pinctrl@... {

  qup_spi0_data_clk: qup-spi0-data-clk {
    pins = "gpio0", "gpio1", "gpio2";
    function = "qup0";

};

  qup_spi0_cs: qup-spi0-cs {
    pins = "gpio3";
    function = "qup0";
  };
  qup_spi0_cs_gpio: qup-spi0-cs-gpio {
    pins = "gpio3";
    function = "gpio";

};
};

arch/arm64/boot/dts/qcom/sc7280.dtsi tlmm: pinctrl@f100000 { · line 1491

Quoted 7 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi +7 -0
+                       qup_uart0_default: qup-uart0-default {
+                               pinmux {
+                                       pins = "gpio0", "gpio1",
+                                              "gpio2", "gpio3";
+                                       function = "qup00";
+                               };
+                       };
Quoted 6 lines

I suspect things would actually be cleaner if we broke the uart lines up since the boards tend to have to adjust pulls differently for each line. With the new "no pinconf / pinmux" world it's pretty clean. It's obviously up to Bjorn, but if it were me I'd request this in the SoC file:

I'd like that.

Quoted 58 lines

qup_uart0_cts: qup-uart0-cts {
pins = "...";
function = "qup00";
};

qup_uart0_rts: qup-uart0-rts {
pins = "...";
function = "qup00";
};

qup_uart0_rx: qup-uart0-rx {
pins = "...";
function = "qup00";
};

qup_uart0_tx: qup-uart0-tx {
pins = "...";
function = "qup00";
};

...and now when the board file wants to adjust the pulls they can just reference each one:

/*
* Comments about why the UART0 pulls make sense.
* Blah blah blah.
*/

&qup_uart0_cts {
bias-pull-down;
};

&qup_uart0_rts {
drive-strength = <2>;
bias-disable;
};

&qup_uart0_rx {
bias-pull-up;
};

&qup_uart0_tx {
drive-strength = <2>;
bias-disable;
};

Quoted 1 line

+ qspi: spi@88dc000 {

I believe the qspi node is sorted incorrectly. When I apply this to the top of the Qualcomm tree it shows up after the "apps_smmu:
iommu@15000000" node. However:

0x088dc000 < 0x15000000

...which means it should be _before_.

-Doug

#3 lore raw
In reply to Doug Anderson

Hi Doug,

Quoted 11 lines · Doug Anderson

Hi,

arch/arm64/boot/dts/qcom/sc7280-idp.dts chosen { · line 70

Quoted 6 lines · Roja Rani Yarubandi

+&qspi_cs0 {

arch/arm64/boot/dts/qcom/sc7280-idp.dts +4 -0
+       pinconf {
+               pins = "gpio15";
+               bias-disable;
+       };
Quoted 6 lines

The "pinconf" / "pinmux" subnode shouldn't be used for new SoCs. See:

http://lore.kernel.org/r/CAD=FV=UY_AFRrAY0tef5jP698LEng6oN652LcX3B4nG=aWh0gA@mail.gmail.com

...same feedback for this whole patch.

Ok, will do the changes.

Quoted 91 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi tlmm: pinctrl@f100000 { · line 1363

Quoted 7 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi +7 -0
+                       qup_spi0_default: qup-spi0-default {
+                               pinmux {
+                                       pins = "gpio0", "gpio1",
+                                              "gpio2", "gpio3";
+                                       function = "qup00";
+                               };
+                       };

Please split these SPI nodes as per the thread above, like:

tlmm: pinctrl@... {

  qup_spi0_data_clk: qup-spi0-data-clk {
    pins = "gpio0", "gpio1", "gpio2";
    function = "qup0";

};

  qup_spi0_cs: qup-spi0-cs {
    pins = "gpio3";
    function = "qup0";
  };
  qup_spi0_cs_gpio: qup-spi0-cs-gpio {
    pins = "gpio3";
    function = "gpio";

};
};

arch/arm64/boot/dts/qcom/sc7280.dtsi tlmm: pinctrl@f100000 { · line 1491

Quoted 7 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi +7 -0
+                       qup_uart0_default: qup-uart0-default {
+                               pinmux {
+                                       pins = "gpio0", "gpio1",
+                                              "gpio2", "gpio3";
+                                       function = "qup00";
+                               };
+                       };

I suspect things would actually be cleaner if we broke the uart lines up since the boards tend to have to adjust pulls differently for each line. With the new "no pinconf / pinmux" world it's pretty clean. It's obviously up to Bjorn, but if it were me I'd request this in the SoC file:

qup_uart0_cts: qup-uart0-cts {
pins = "...";
function = "qup00";
};

qup_uart0_rts: qup-uart0-rts {
pins = "...";
function = "qup00";
};

qup_uart0_rx: qup-uart0-rx {
pins = "...";
function = "qup00";
};

qup_uart0_tx: qup-uart0-tx {
pins = "...";
function = "qup00";
};

...and now when the board file wants to adjust the pulls they can just reference each one:

/*
* Comments about why the UART0 pulls make sense.
* Blah blah blah.
*/

&qup_uart0_cts {
bias-pull-down;
};

&qup_uart0_rts {
drive-strength = <2>;
bias-disable;
};

&qup_uart0_rx {
bias-pull-up;
};

&qup_uart0_tx {
drive-strength = <2>;
bias-disable;
};

Quoted 1 line

+ qspi: spi@88dc000 {

Quoted 8 lines

I believe the qspi node is sorted incorrectly. When I apply this to the top of the Qualcomm tree it shows up after the "apps_smmu:
iommu@15000000" node. However:

0x088dc000 < 0x15000000

...which means it should be _before_.

Sure, will move before apps_smmu

Thanks,
Roja

Quoted 1 line

-Doug