diff mbox series

[V7,1/7] arm64: dts: sc7280: Add QSPI node

Message ID 1630643340-10373-2-git-send-email-rajpat@codeaurora.org (mailing list archive)
State Superseded
Headers show
Series Add QSPI and QUPv3 DT nodes for SC7280 SoC | expand

Commit Message

Rajesh Patil Sept. 3, 2021, 4:28 a.m. UTC
From: Roja Rani Yarubandi <rojay@codeaurora.org>

Add QSPI DT node and qspi_opp_table for SC7280 SoC.

Move qspi_opp_table to / because SPI nodes assume
any child node is a spi device and so we can't put the
table underneath the spi controller.

Signed-off-by: Roja Rani Yarubandi <rojay@codeaurora.org>
Signed-off-by: Rajesh Patil <rajpat@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 61 ++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

Comments

Matthias Kaehlcke Sept. 3, 2021, 3:54 p.m. UTC | #1
On Fri, Sep 03, 2021 at 09:58:54AM +0530, Rajesh Patil wrote:
> From: Roja Rani Yarubandi <rojay@codeaurora.org>
> 
> Add QSPI DT node and qspi_opp_table for SC7280 SoC.
> 
> Move qspi_opp_table to / because SPI nodes assume
> any child node is a spi device and so we can't put the
> table underneath the spi controller.
> 
> Signed-off-by: Roja Rani Yarubandi <rojay@codeaurora.org>
> Signed-off-by: Rajesh Patil <rajpat@codeaurora.org>
> ---

It would be really helpful for reviewers to have a change log,
especially for the larger patches of the series. You had one
for some versions and then dropped it :(

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Doug Anderson Sept. 3, 2021, 4:28 p.m. UTC | #2
Hi,

On Thu, Sep 2, 2021 at 9:29 PM Rajesh Patil <rajpat@codeaurora.org> wrote:
>
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -415,6 +415,25 @@
>                 method = "smc";
>         };
>
> +       qspi_opp_table: qspi-opp-table {
> +               compatible = "operating-points-v2";
> +
> +               opp-75000000 {
> +                       opp-hz = /bits/ 64 <75000000>;
> +                       required-opps = <&rpmhpd_opp_low_svs>;
> +               };
> +
> +               opp-150000000 {
> +                       opp-hz = /bits/ 64 <150000000>;
> +                       required-opps = <&rpmhpd_opp_svs>;
> +               };
> +

Any chance you could add a 200 MHz OPP point? It seems plausible that
we might want to run the Quad SPI bus at 50 MHz and this OPP needs to
be 4x that, so 200 MHz. ...or does it magically handle that case by
one of the other OPPs?

> +               opp-300000000 {
> +                       opp-hz = /bits/ 64 <300000000>;
> +                       required-opps = <&rpmhpd_opp_nom>;
> +               };
> +       };
> +
>         soc: soc@0 {
>                 #address-cells = <2>;
>                 #size-cells = <2>;
> @@ -1318,6 +1337,23 @@
>                         };
>                 };
>
> +               qspi: spi@88dc000 {
> +                       compatible = "qcom,qspi-v1";

The above compatible should be:

compatible = "qcom,sdm7280-qspi", "qcom,qspi-v1";

...and you should fix the devicetree bindings to handle that. You
should also fix sc7180.

Technically the "qcom,sdm7280-qspi" isn't really needed to make
anything work today but having it is encouraged so that if we need to
deal with a quirk in the future we can easily do it. Also note that
your current dts will cause a bindings error because the current
bindings _require_ you to have two compatible strings.
Rajesh Patil Sept. 16, 2021, 4:31 a.m. UTC | #3
On 2021-09-03 21:58, Doug Anderson wrote:
> Hi,
> 
> On Thu, Sep 2, 2021 at 9:29 PM Rajesh Patil <rajpat@codeaurora.org> 
> wrote:
>> 
>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> @@ -415,6 +415,25 @@
>>                 method = "smc";
>>         };
>> 
>> +       qspi_opp_table: qspi-opp-table {
>> +               compatible = "operating-points-v2";
>> +
>> +               opp-75000000 {
>> +                       opp-hz = /bits/ 64 <75000000>;
>> +                       required-opps = <&rpmhpd_opp_low_svs>;
>> +               };
>> +
>> +               opp-150000000 {
>> +                       opp-hz = /bits/ 64 <150000000>;
>> +                       required-opps = <&rpmhpd_opp_svs>;
>> +               };
>> +
> 
> Any chance you could add a 200 MHz OPP point? It seems plausible that
> we might want to run the Quad SPI bus at 50 MHz and this OPP needs to
> be 4x that, so 200 MHz. ...or does it magically handle that case by
> one of the other OPPs?

Okay

> 
>> +               opp-300000000 {
>> +                       opp-hz = /bits/ 64 <300000000>;
>> +                       required-opps = <&rpmhpd_opp_nom>;
>> +               };
>> +       };
>> +
>>         soc: soc@0 {
>>                 #address-cells = <2>;
>>                 #size-cells = <2>;
>> @@ -1318,6 +1337,23 @@
>>                         };
>>                 };
>> 
>> +               qspi: spi@88dc000 {
>> +                       compatible = "qcom,qspi-v1";
> 
> The above compatible should be:
> 
> compatible = "qcom,sdm7280-qspi", "qcom,qspi-v1";
> 
> ...and you should fix the devicetree bindings to handle that. You
> should also fix sc7180.
> 
> Technically the "qcom,sdm7280-qspi" isn't really needed to make
> anything work today but having it is encouraged so that if we need to
> deal with a quirk in the future we can easily do it. Also note that
> your current dts will cause a bindings error because the current
> bindings _require_ you to have two compatible strings.

Okay
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 53a21d0..7ec9871 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -415,6 +415,25 @@ 
 		method = "smc";
 	};
 
+	qspi_opp_table: qspi-opp-table {
+		compatible = "operating-points-v2";
+
+		opp-75000000 {
+			opp-hz = /bits/ 64 <75000000>;
+			required-opps = <&rpmhpd_opp_low_svs>;
+		};
+
+		opp-150000000 {
+			opp-hz = /bits/ 64 <150000000>;
+			required-opps = <&rpmhpd_opp_svs>;
+		};
+
+		opp-300000000 {
+			opp-hz = /bits/ 64 <300000000>;
+			required-opps = <&rpmhpd_opp_nom>;
+		};
+	};
+
 	soc: soc@0 {
 		#address-cells = <2>;
 		#size-cells = <2>;
@@ -1318,6 +1337,23 @@ 
 			};
 		};
 
+		qspi: spi@88dc000 {
+			compatible = "qcom,qspi-v1";
+			reg = <0 0x088dc000 0 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&gcc GCC_QSPI_CNOC_PERIPH_AHB_CLK>,
+				 <&gcc GCC_QSPI_CORE_CLK>;
+			clock-names = "iface", "core";
+			interconnects = <&gem_noc MASTER_APPSS_PROC 0
+					&cnoc2 SLAVE_QSPI_0 0>;
+			interconnect-names = "qspi-config";
+			power-domains = <&rpmhpd SC7280_CX>;
+			operating-points-v2 = <&qspi_opp_table>;
+			status = "disabled";
+		};
+
 		dc_noc: interconnect@90e0000 {
 			reg = <0 0x090e0000 0 0x5080>;
 			compatible = "qcom,sc7280-dc-noc";
@@ -1513,6 +1549,31 @@ 
 			gpio-ranges = <&tlmm 0 0 175>;
 			wakeup-parent = <&pdc>;
 
+			qspi_clk: qspi-clk {
+				pins = "gpio14";
+				function = "qspi_clk";
+			};
+
+			qspi_cs0: qspi-cs0 {
+				pins = "gpio15";
+				function = "qspi_cs";
+			};
+
+			qspi_cs1: qspi-cs1 {
+				pins = "gpio19";
+				function = "qspi_cs";
+			};
+
+			qspi_data01: qspi-data01 {
+				pins = "gpio12", "gpio13";
+				function = "qspi_data";
+			};
+
+			qspi_data12: qspi-data12 {
+				pins = "gpio16", "gpio17";
+				function = "qspi_data";
+			};
+
 			qup_uart5_default: qup-uart5-default {
 				pins = "gpio46", "gpio47";
 				function = "qup13";