Re: [PATCH v1 1/3] ARM: MR26: MR32: remove bogus nand-ecc-algo property

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3.06.2023 16:16, Christian Lamparter wrote:
| bcm53015-meraki-mr26.dtb: nand-controller@18028000:
|   nand@0:nand-ecc-algo:0: 'hw' is not one of ['hamming', 'bch', 'rs']
| From schema: Documentation/[...]/nand-controller.yaml
| bcm53016-meraki-mr32.dtb: nand-controller@18028000:
|   nand@0:nand-ecc-algo:0: 'hw' is not one of ['hamming', 'bch', 'rs']
| From schema: Documentation/[...]/nand-controller.yaml

original ECC values for these old Merakis are sadly not
provided by the vendor. It looks like Meraki just stuck
with what Broadcom's SDK was doing... which left it up
to their proprietary nand driver.

It's clear at least that they used the hardware's ecc
engine, so update the device-tree file accordingly to
specify the nand-controller as the ecc-engine.

I believe that initial state can be "setup" at hardware level. I believe
Broadcom's bootloader and their SDK driver just reads current ECC setup
(which goes down to the hardware level).

Years ago I proposed change for brcmnand to do the same (which was
apparently a bad idea):
[PATCH] mtd: brcmnand: set initial ECC params based on info from HW
https://lists.infradead.org/pipermail/linux-mtd/2016-February/065314.html

That said I think it still should be possible to determine what algo is
used and specify that in DT.


this patch also removes the partition index numbers
from the MR32's partition node-names and does some
whitespace removal in order to fit the comment about
the partition oddities into the 100 characters per
limit.

Fixes: 935327a73553 ("ARM: dts: BCM5301X: Add DT for Meraki MR26")
Fixes: ec88a9c344d9 ("ARM: BCM5301X: Add DT for Meraki MR32")
Reported-by: Rafał Miłecki <zajec5@xxxxxxxxx> (via mail)
Signed-off-by: Christian Lamparter <chunkeey@xxxxxxxxx>

mr32
---
  arch/arm/boot/dts/bcm53015-meraki-mr26.dts | 68 +++++++++--------
  arch/arm/boot/dts/bcm53016-meraki-mr32.dts | 88 ++++++++++++----------
  2 files changed, 86 insertions(+), 70 deletions(-)

diff --git a/arch/arm/boot/dts/bcm53015-meraki-mr26.dts b/arch/arm/boot/dts/bcm53015-meraki-mr26.dts
index a2eee9a1e5a7..9ea4ffc1bb71 100644
--- a/arch/arm/boot/dts/bcm53015-meraki-mr26.dts
+++ b/arch/arm/boot/dts/bcm53015-meraki-mr26.dts
@@ -9,7 +9,6 @@
  /dts-v1/;
#include "bcm4708.dtsi"
-#include "bcm5301x-nand-cs0-bch8.dtsi"
  #include <dt-bindings/leds/common.h>
/ {
@@ -73,41 +72,50 @@ &gmac3 {
  	status = "disabled";
  };
-&nandcs {
-	nand-ecc-algo = "hw";
+&nand_controller {
+	nand@0 {
+		compatible = "brcm,nandcs";
+		reg = <0>;
+		#address-cells = <1>;
+		#size-cells = <1>;
- partitions {
-		compatible = "fixed-partitions";
-		#address-cells = <0x1>;
-		#size-cells = <0x1>;
+		nand-ecc-engine = <&nand_controller>;
+		nand-ecc-strength = <8>;
+		nand-ecc-step-size = <512>;

If we really can't determine ECC algo maybe we could still have sth like
bcm5301x-nand-cs0-FOO.dtsi
to match other ECC setup?

That way you probably also shouldn't need &nand_controller here.


- partition@0 {
-			label = "u-boot";
-			reg = <0x0 0x200000>;
-			read-only;
-		};
+		partitions {
+			compatible = "fixed-partitions";
+			#address-cells = <0x1>;
+			#size-cells = <0x1>;
- partition@200000 {
-			label = "u-boot-env";
-			reg = <0x200000 0x200000>;
-			/* empty */
-		};
+			partition@0 {
+				label = "u-boot";
+				reg = <0x0 0x200000>;
+				read-only;
+			};
- partition@400000 {
-			label = "u-boot-backup";
-			reg = <0x400000 0x200000>;
-			/* empty */
-		};
+			partition@200000 {
+				label = "u-boot-env";
+				reg = <0x200000 0x200000>;
+				/* empty */
+			};
- partition@600000 {
-			label = "u-boot-env-backup";
-			reg = <0x600000 0x200000>;
-			/* empty */
-		};
+			partition@400000 {
+				label = "u-boot-backup";
+				reg = <0x400000 0x200000>;
+				/* empty */
+			};
- partition@800000 {
-			label = "ubi";
-			reg = <0x800000 0x7780000>;
+			partition@600000 {
+				label = "u-boot-env-backup";
+				reg = <0x600000 0x200000>;
+				/* empty */
+			};
+
+			partition@800000 {
+				label = "ubi";
+				reg = <0x800000 0x7780000>;
+			};
  		};
  	};
  };
diff --git a/arch/arm/boot/dts/bcm53016-meraki-mr32.dts b/arch/arm/boot/dts/bcm53016-meraki-mr32.dts
index b6a066f949ad..bca39b30ace8 100644
--- a/arch/arm/boot/dts/bcm53016-meraki-mr32.dts
+++ b/arch/arm/boot/dts/bcm53016-meraki-mr32.dts
@@ -9,7 +9,6 @@
  /dts-v1/;
#include "bcm4708.dtsi"
-#include "bcm5301x-nand-cs0-bch8.dtsi"
  #include <dt-bindings/leds/common.h>
/ {
@@ -124,49 +123,58 @@ &pwm {
  	pinctrl-0 = <&pinmux_pwm>;
  };
-&nandcs {
-	nand-ecc-algo = "hw";
-
-	partitions {
-		/*
-		 * The partition autodetection does not work for this device.
-		 * It will only detect the "nvram" partition with an incorrect size.
-		 *	[    1.721667] 1 bcm47xxpart partitions found on MTD device brcmnand.0
-		 *	[    1.727962] Creating 1 MTD partitions on "brcmnand.0":
-		 *	[    1.733117] 0x000000400000-0x000008000000 : "nvram"
-		 */
-
-		compatible = "fixed-partitions";
-		#address-cells = <0x1>;
-		#size-cells = <0x1>;
-
-		partition0@0 {
-			label = "u-boot";
-			reg = <0x0 0x100000>;
-			read-only;
-		};
+&nand_controller {
+	nand@0 {
+		compatible = "brcm,nandcs";
+		reg = <0>;
+		#address-cells = <1>;
+		#size-cells = <1>;
- partition1@100000 {
-			label = "bootkernel1";
-			reg = <0x100000 0x300000>;
-			read-only;
-		};
+		nand-ecc-engine = <&nand_controller>;
+		nand-ecc-strength = <8>;
+		nand-ecc-step-size = <512>;
+
+		partitions {
+			/*
+			 * The partition autodetection does not work for this device.
+			 * It will only detect the "nvram" partition with an incorrect size.
+			 *	[ 1.721667] 1 bcm47xxpart partitions found on MTD device brcmnand.0
+			 *	[ 1.727962] Creating 1 MTD partitions on "brcmnand.0":
+			 *	[ 1.733117] 0x000000400000-0x000008000000 : "nvram"
+			 */
+
+			compatible = "fixed-partitions";
+			#address-cells = <0x1>;
+			#size-cells = <0x1>;
+
+			partition@0 {
+				label = "u-boot";
+				reg = <0x0 0x100000>;
+				read-only;
+			};
- partition2@400000 {
-			label = "nvram";
-			reg = <0x400000 0x100000>;
-			read-only;
-		};
+			partition@100000 {
+				label = "bootkernel1";
+				reg = <0x100000 0x300000>;
+				read-only;
+			};
- partition3@500000 {
-			label = "bootkernel2";
-			reg = <0x500000 0x300000>;
-			read-only;
-		};
+			partition@400000 {
+				label = "nvram";
+				reg = <0x400000 0x100000>;
+				read-only;
+			};
- partition4@800000 {
-			label = "ubi";
-			reg = <0x800000 0x7780000>;
+			partition@500000 {
+				label = "bootkernel2";
+				reg = <0x500000 0x300000>;
+				read-only;
+			};
+
+			partition@800000 {
+				label = "ubi";
+				reg = <0x800000 0x7780000>;
+			};
  		};
  	};
  };




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux