[PATCH v1 1/3] ARM: MR26: MR32: remove bogus nand-ecc-algo property
Rafał Miłecki
zajec5 at gmail.com
Mon Jun 5 04:19:40 PDT 2023
On 3.06.2023 16:16, Christian Lamparter wrote:
> | bcm53015-meraki-mr26.dtb: nand-controller at 18028000:
> | nand at 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 at 18028000:
> | nand at 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 at gmail.com> (via mail)
> Signed-off-by: Christian Lamparter <chunkeey at gmail.com>
>
> 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 at 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 at 0 {
> - label = "u-boot";
> - reg = <0x0 0x200000>;
> - read-only;
> - };
> + partitions {
> + compatible = "fixed-partitions";
> + #address-cells = <0x1>;
> + #size-cells = <0x1>;
>
> - partition at 200000 {
> - label = "u-boot-env";
> - reg = <0x200000 0x200000>;
> - /* empty */
> - };
> + partition at 0 {
> + label = "u-boot";
> + reg = <0x0 0x200000>;
> + read-only;
> + };
>
> - partition at 400000 {
> - label = "u-boot-backup";
> - reg = <0x400000 0x200000>;
> - /* empty */
> - };
> + partition at 200000 {
> + label = "u-boot-env";
> + reg = <0x200000 0x200000>;
> + /* empty */
> + };
>
> - partition at 600000 {
> - label = "u-boot-env-backup";
> - reg = <0x600000 0x200000>;
> - /* empty */
> - };
> + partition at 400000 {
> + label = "u-boot-backup";
> + reg = <0x400000 0x200000>;
> + /* empty */
> + };
>
> - partition at 800000 {
> - label = "ubi";
> - reg = <0x800000 0x7780000>;
> + partition at 600000 {
> + label = "u-boot-env-backup";
> + reg = <0x600000 0x200000>;
> + /* empty */
> + };
> +
> + partition at 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 at 0 {
> - label = "u-boot";
> - reg = <0x0 0x100000>;
> - read-only;
> - };
> +&nand_controller {
> + nand at 0 {
> + compatible = "brcm,nandcs";
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <1>;
>
> - partition1 at 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 at 0 {
> + label = "u-boot";
> + reg = <0x0 0x100000>;
> + read-only;
> + };
>
> - partition2 at 400000 {
> - label = "nvram";
> - reg = <0x400000 0x100000>;
> - read-only;
> - };
> + partition at 100000 {
> + label = "bootkernel1";
> + reg = <0x100000 0x300000>;
> + read-only;
> + };
>
> - partition3 at 500000 {
> - label = "bootkernel2";
> - reg = <0x500000 0x300000>;
> - read-only;
> - };
> + partition at 400000 {
> + label = "nvram";
> + reg = <0x400000 0x100000>;
> + read-only;
> + };
>
> - partition4 at 800000 {
> - label = "ubi";
> - reg = <0x800000 0x7780000>;
> + partition at 500000 {
> + label = "bootkernel2";
> + reg = <0x500000 0x300000>;
> + read-only;
> + };
> +
> + partition at 800000 {
> + label = "ubi";
> + reg = <0x800000 0x7780000>;
> + };
> };
> };
> };
More information about the linux-arm-kernel
mailing list