On 10/30/19 4:27 AM, Daniel Danzberger wrote:

This device contains 2 flash devices. One NOR (32M) and one NAND (128M).
U-boot and caldata are on the NOR, the firmware on the NAND.

     SoC:    IPQ4019
     CPU:    4x 710MHz ARMv7
     RAM:    256MB
     FLASH:  NOR:32MB NAND:128MB

[...]



 .../arch/arm/boot/dts/qcom-ipq4019-bus.dtsi   | 1142 +++++++++++++++++
 .../include/dt-bindings/msm/msm-bus-ids.h     |  869 +++++++++++++

The sudden appearance of a need the MSM bus and its IDs worries me.

With 25 devices already on the ipq40xx platform without them, it feels
like something is missing if they are needed by this one.


diff --git a/target/linux/ipq40xx/config-4.19 b/target/linux/ipq40xx/config-4.19
index 8948b73ff7..3ee921abed 100644
--- a/target/linux/ipq40xx/config-4.19
+++ b/target/linux/ipq40xx/config-4.19
@@ -303,6 +303,9 @@ CONFIG_MTD_NAND_ECC=y
  CONFIG_MTD_NAND_QCOM=y
  CONFIG_MTD_SPI_NAND=y
  CONFIG_MTD_SPI_NOR=y
+CONFIG_MTD_SPINAND_MT29F=y
+CONFIG_MTD_SPINAND_GIGADEVICE=y
+CONFIG_MTD_SPINAND_ONDIEECC=y


The CONFIG_SPINAND_* additions are not required for upstream SPI-NAND


  CONFIG_MTD_SPLIT_FIRMWARE=y
  CONFIG_MTD_SPLIT_FIT_FW=y
  CONFIG_MTD_UBI=y

[...]

diff --git 
a/target/linux/ipq40xx/files-4.19/arch/arm/boot/dts/qcom-ipq4019-wpj419.dts 
b/target/linux/ipq40xx/files-4.19/arch/arm/boot/dts/qcom-ipq4019-wpj419.dts
new file mode 100644
index 0000000000..5553bbd166
--- /dev/null
+++ b/target/linux/ipq40xx/files-4.19/arch/arm/boot/dts/qcom-ipq4019-wpj419.dts
@@ -0,0 +1,371 @@
+/* Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2019, Nguyen Dinh Phi <phi_ngu...@compex.com.sg>
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ */
+

[...]

+
+               spi_0: spi@78b5000 {
+                       pinctrl-0 = <&spi_0_pins>;
+                       pinctrl-names = "default";
+                       status = "okay";
+                       cs-gpios = <&tlmm 12 GPIO_ACTIVE_HIGH>, <&tlmm 41 
GPIO_ACTIVE_HIGH>;
+                       num-cs = <2>;
+
+                       m25p80@0 {
+                               #address-cells = <1>;
+                               #size-cells = <1>;
+                               reg = <0>;
+                               linux,modalias = "m25p80", "n25q128a11";
+                               compatible = "jedec,spi-nor", "n25q128a11";
+                               spi-max-frequency = <24000000>;


I don't think you need linux,modalias here, nor the chip type in the compatible line.
I believe that the following compatible line is sufficient

    compatible = "jedec,spi-nor";


You might also want to consider "flash@0" or "nor@0" or "nor_flash@0",
or the like, rather than a chip-specific name. (I'm not a committer.)


+
+                               partitions {
+                                       compatible = "fixed-partitions";
+
+                                       partition@0 {
+                                               label = "0:SBL1";
+                                               reg = <0x000000 0x040000>;
+                                               read-only;
+                                       };
+
+                                       partition@40000 {
+                                               label = "0:MIBIB";
+                                               reg = <0x040000 0x020000>;
+                                               read-only;
+                                       };
+
+                                       partition@60000 {
+                                               label = "0:QSEE";
+                                               reg = <0x060000 0x060000>;
+                                               read-only;
+                                       };
+
+                                       partition@c0000 {
+                                               label = "0:CDT";
+                                               reg = <0x0c0000 0x010000>;
+                                               read-only;
+                                       };


Someone may rip on you for capitalization of labels. (I'm not a committer.)


+
+                                       partition@d0000 {
+                                               label = "0:DDRPARAMS";
+                                               reg = <0x0d0000 0x010000>;
+                                               read-only;
+                                       };
+
+                                       partition@e0000 {
+                                               label = "u-boot-env";
+                                               reg = <0x0e0000 0x010000>;
+                                               read-only;
+                                       };


U-Boot environment may want/need to be writable


+
+                                       partition@f0000 {
+                                               label = "u-boot";
+                                               reg = <0x0f0000 0x080000>;
+                                               read-only;
+                                       };
+
+                                       partition@170000 {
+                                               label = "art";
+                                               reg = <0x170000 0x010000>;
+                                               read-only;
+                                       };
+                               };
+                       };
+
+                       mt29f@1 {
+                               #address-cells = <1>;
+                               #size-cells = <1>;
+                               reg = <1>;
+                               status = "okay";
+                               compatible = "spinand,mt29f";
+                               spi-max-frequency = <24000000>;


Same comment on "mt29f" vs. something generic and descriptive.


Converting to the upstream SPI-NAND driver here should be as simple as

    compatible = "spi-nand";



+
+                               partitions {
+                                       compatible = "fixed-partitions";
+
+                                       partition@0 {
+                                               label = "ubi";
+                                               reg = <0x0000000 0x8000000>;
+                                       };
+                               };
+                       };
+               };
+
[...]


_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Reply via email to