arm64: dts: qcom: sc7280: Fix qspi pin config
authorDouglas Anderson <dianders@chromium.org>
Thu, 23 Mar 2023 17:30:17 +0000 (10:30 -0700)
committerBjorn Andersson <andersson@kernel.org>
Fri, 7 Apr 2023 17:54:09 +0000 (10:54 -0700)
Similar to sc7180 (see the patch ("arm64: dts: qcom: sc7180: Fix
trogdor qspi pin config")), we should adjust the qspi pin config for
sc7280.

I won't re-describe all the research/arguments in the sc7180 patch
here, but there are a few differences for sc7280 worth noting:

1. On herobrine the SPI flash (qspi) is wired up differently on the
   board. Rather than Cr50 and the AP being wired directly together,
   there's actually a mux that will _either_ connect the AP to the
   flash or Cr50 to the flash. This means that the internal pulls on
   Cr50 don't affect us and we should enable our own pulldowns.

2. On herobrine, EEs added an external pulldown on the MISO line. The
   argument in the schematic said that we added it (but not one on
   MOSI and CLK) because Cr50 already enabled pulldowns on MOSI and
   CLK. ...though, as per #1, those Cr50 pulldowns would only affect
   the line when the mux was swung to Cr50.

The ironic result of #1 and #2 is that the external pulldowns on
CLK/MISO/MOSI on herobrine are _exactly opposite_ of the ones on
trogdor.

3. While I still don't have the actual exact schematics for all
   variants of IDP/CRD that were produced, I have some reference
   schematics that give me a belief of how the qspi is hooked up
   there. From this, I'm fairly certain that all of the older variants
   of IDP/CRD either have a pulldown on the CLK/MOSI/MISO lines (maybe
   through a direct connect to Cr50) or have no pull (in other words,
   they don't have a pullup). I'll go ahead and enable internal
   pulldowns on all the lines since that won't hurt to double-pull if
   there's an external pulldown and it's nice to have a pulldown if
   there's nothing external. Note that this only affects _older_
   CRDs. Newer revs are considered "herobrine" (see the hoglin/zoglin
   device trees).

4. I didn't find the same strange "auto-switch-to-keeper" at suspend
   when probing on sc7280. Whatever pulls (or lack thereof) I left at
   suspend time seemed to persist into suspend.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230323102605.13.Ib44c3e417c414a4227db8def75ded37ad368212c@changeid
arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
arch/arm64/boot/dts/qcom/sc7280.dtsi

index 16fb20369c014dcad8a9d989056e46897786fdce..f562e4d2b6552c4922ffdafeb669285063e60b21 100644 (file)
@@ -60,8 +60,9 @@
  */
 &qspi {
        status = "okay";
-       pinctrl-names = "default";
-       pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>;
+       pinctrl-names = "default", "sleep";
+       pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data0>, <&qspi_data1>;
+       pinctrl-1 = <&qspi_sleep>;
 
        spi_flash: flash@0 {
                compatible = "jedec,spi-nor";
                iommus = <&apps_smmu 0x1c02 0x1>;
        };
 };
+
+/* PINCTRL - chrome-common pinctrl */
+
+&tlmm {
+       qspi_sleep: qspi-sleep-state {
+               pins = "gpio12", "gpio13", "gpio14", "gpio15";
+
+               /*
+                * When we're not actively transferring we want pins as GPIOs
+                * with output disabled so that the quad SPI IP block stops
+                * driving them. We rely on the normal pulls configured in
+                * the active state and don't redefine them here. Also note
+                * that we don't need the reverse (output-enable) in the
+                * normal mode since the "output-enable" only matters for
+                * GPIO function.
+                */
+               function = "gpio";
+               output-disable;
+       };
+};
index b6137816f2f3d2e3ca832010e4e98f9de896f985..e651f633341f51f59982447a3b76723dc829a2c9 100644 (file)
@@ -692,18 +692,22 @@ ap_ec_spi: &spi10 {
 };
 
 &qspi_cs0 {
-       bias-disable;
+       bias-disable;           /* External pullup */
        drive-strength = <8>;
 };
 
 &qspi_clk {
-       bias-disable;
+       bias-pull-down;         /* No external pulls */
        drive-strength = <8>;
 };
 
-&qspi_data01 {
-       /* High-Z when no transfers; nice to park the lines */
-       bias-pull-up;
+&qspi_data0 {
+       bias-pull-down;         /* No external pulls */
+       drive-strength = <8>;
+};
+
+&qspi_data1 {
+       bias-disable;           /* External pulldown */
        drive-strength = <8>;
 };
 
index 5dc9bee28e7f6a59bf84c19951c28d5a1d66229b..c6dc200c00cea42c891e130beed7fe74522e220f 100644 (file)
 };
 
 &qspi_cs0 {
-       bias-disable;
+       bias-disable;           /* External pullup */
 };
 
 &qspi_clk {
-       bias-disable;
+       bias-pull-down;         /* No external pulls or external pulldown */
 };
 
-&qspi_data01 {
-       /* High-Z when no transfers; nice to park the lines */
-       bias-pull-up;
+&qspi_data0 {
+       bias-pull-down;         /* No external pulls or external pulldown */
+};
+
+&qspi_data1 {
+       bias-pull-down;         /* No external pulls or external pulldown */
 };
 
 &qup_uart5_tx {
index 18a4b097ebbd880ac5e11f2d3e1c97cd40eb5f85..0d22fe42b133fdd08028966e9f92adf139ab8800 100644 (file)
                                function = "qspi_cs";
                        };
 
-                       qspi_data01: qspi-data01-state {
-                               pins = "gpio12", "gpio13";
+                       qspi_data0: qspi-data0-state {
+                               pins = "gpio12";
+                               function = "qspi_data";
+                       };
+
+                       qspi_data1: qspi-data1-state {
+                               pins = "gpio13";
                                function = "qspi_data";
                        };