[PATCH] mtd: aspeed-smc: improve probe resilience

[PATCH] mtd: aspeed-smc: improve probe resilience

[Patrick Williams]

The aspeed-smc can have multiple SPI devices attached to it in the
device tree.  If one of the devices is missing or failing the entire
probe will fail and all MTD devices under the controller will be
removed.  On OpenBMC this results in a kernel panic due to missing
rootfs:

[    0.538774] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
[    0.540471] aspeed-smc 1e620000.spi: w25q01jv-iq (131072 Kbytes)
[    0.540750] aspeed-smc 1e620000.spi: CE0 window [ 0x20000000 - 0x28000000 ] 128MB
[    0.540943] aspeed-smc 1e620000.spi: CE1 window [ 0x28000000 - 0x2c000000 ] 64MB
[    0.541143] aspeed-smc 1e620000.spi: read control register: 203b0041
[    0.581442] 5 fixed-partitions partitions found on MTD device bmc
[    0.581625] Creating 5 MTD partitions on "bmc":
[    0.581854] 0x000000000000-0x0000000e0000 : "u-boot"
[    0.584472] 0x0000000e0000-0x000000100000 : "u-boot-env"
[    0.586468] 0x000000100000-0x000000a00000 : "kernel"
[    0.588465] 0x000000a00000-0x000006000000 : "rofs"
[    0.590552] 0x000006000000-0x000008000000 : "rwfs"
[    0.592605] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
[    0.592801] aspeed-smc 1e620000.spi: unrecognized JEDEC id bytes: 00 00 00 00 00 00
[    0.593039] Deleting MTD partitions on "bmc":
[    0.593175] Deleting u-boot MTD partition
[    0.637929] Deleting u-boot-env MTD partition
[    0.829527] Deleting kernel MTD partition
[    0.856902] Freeing initrd memory: 1032K
[    0.866428] Deleting rofs MTD partition
[    0.906264] Deleting rwfs MTD partition
[    0.986628] aspeed-smc 1e620000.spi: Aspeed SMC probe failed -2
[    0.986929] aspeed-smc: probe of 1e620000.spi failed with error -2
...
[    2.936719] /dev/mtdblock: Can't open blockdev
mount: mounting /dev/mtdblock on run/initramfs/ro failed: No such file or directory
[    2.963030] MTD: Couldn't look up '/dev/mtdblock': -2
mount: mounting /dev/mtdblock on run/initramfs/rw failed: No such file or directory

Mounting read-write /dev/mtdblock filesystem failed.  Please fix and run
	mount /dev/mtdblock run/initramfs/rw -t jffs2 -o rw
or perform a factory reset with the clean-rwfs-filesystem option.
Fatal error, triggering kernel panic!
[    3.013047] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000100

Many BMC designs have two flash chips so that they can handle a hardware
failure of one of them.  If one chip failed, it doesn't do any good to
have redundancy if they all get removed anyhow.

Improve the resilience of the probe function to handle one of the
children being missing or failed.  Only in the case where all children
fail to probe should the controller be failed out.

Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
---
 drivers/mtd/spi-nor/controllers/aspeed-smc.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/controllers/aspeed-smc.c b/drivers/mtd/spi-nor/controllers/aspeed-smc.c
index 7225870e8b18..acfe010f9dd7 100644
--- a/drivers/mtd/spi-nor/controllers/aspeed-smc.c
+++ b/drivers/mtd/spi-nor/controllers/aspeed-smc.c
@@ -769,6 +769,7 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
 	struct device_node *child;
 	unsigned int cs;
 	int ret = -ENODEV;
+	bool found_one = false;
 
 	for_each_available_child_of_node(np, child) {
 		struct aspeed_smc_chip *chip;
@@ -827,8 +828,17 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
 		 * by of property.
 		 */
 		ret = spi_nor_scan(nor, NULL, &hwcaps);
-		if (ret)
-			break;
+		/*
+		 * If we fail to scan the device it might not be present or
+		 * broken.  Don't fail the whole controller if others work.
+		 */
+		if (ret) {
+			if (found_one)
+				ret = 0;
+
+			devm_kfree(controller->dev, chip);
+			continue;
+		}
 
 		ret = aspeed_smc_chip_setup_finish(chip);
 		if (ret)
@@ -839,6 +849,7 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
 			break;
 
 		controller->chips[cs] = chip;
+		found_one = true;
 	}
 
 	if (ret) {
-- 
2.32.0

[PATCH] mtd: aspeed-smc: improve probe resilience

[Patrick Williams]

The aspeed-smc can have multiple SPI devices attached to it in the
device tree.  If one of the devices is missing or failing the entire
probe will fail and all MTD devices under the controller will be
removed.  On OpenBMC this results in a kernel panic due to missing
rootfs:

[    0.538774] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
[    0.540471] aspeed-smc 1e620000.spi: w25q01jv-iq (131072 Kbytes)
[    0.540750] aspeed-smc 1e620000.spi: CE0 window [ 0x20000000 - 0x28000000 ] 128MB
[    0.540943] aspeed-smc 1e620000.spi: CE1 window [ 0x28000000 - 0x2c000000 ] 64MB
[    0.541143] aspeed-smc 1e620000.spi: read control register: 203b0041
[    0.581442] 5 fixed-partitions partitions found on MTD device bmc
[    0.581625] Creating 5 MTD partitions on "bmc":
[    0.581854] 0x000000000000-0x0000000e0000 : "u-boot"
[    0.584472] 0x0000000e0000-0x000000100000 : "u-boot-env"
[    0.586468] 0x000000100000-0x000000a00000 : "kernel"
[    0.588465] 0x000000a00000-0x000006000000 : "rofs"
[    0.590552] 0x000006000000-0x000008000000 : "rwfs"
[    0.592605] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
[    0.592801] aspeed-smc 1e620000.spi: unrecognized JEDEC id bytes: 00 00 00 00 00 00
[    0.593039] Deleting MTD partitions on "bmc":
[    0.593175] Deleting u-boot MTD partition
[    0.637929] Deleting u-boot-env MTD partition
[    0.829527] Deleting kernel MTD partition
[    0.856902] Freeing initrd memory: 1032K
[    0.866428] Deleting rofs MTD partition
[    0.906264] Deleting rwfs MTD partition
[    0.986628] aspeed-smc 1e620000.spi: Aspeed SMC probe failed -2
[    0.986929] aspeed-smc: probe of 1e620000.spi failed with error -2
...
[    2.936719] /dev/mtdblock: Can't open blockdev
mount: mounting /dev/mtdblock on run/initramfs/ro failed: No such file or directory
[    2.963030] MTD: Couldn't look up '/dev/mtdblock': -2
mount: mounting /dev/mtdblock on run/initramfs/rw failed: No such file or directory

Mounting read-write /dev/mtdblock filesystem failed.  Please fix and run
	mount /dev/mtdblock run/initramfs/rw -t jffs2 -o rw
or perform a factory reset with the clean-rwfs-filesystem option.
Fatal error, triggering kernel panic!
[    3.013047] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000100

Many BMC designs have two flash chips so that they can handle a hardware
failure of one of them.  If one chip failed, it doesn't do any good to
have redundancy if they all get removed anyhow.

Improve the resilience of the probe function to handle one of the
children being missing or failed.  Only in the case where all children
fail to probe should the controller be failed out.

Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
---
 drivers/mtd/spi-nor/controllers/aspeed-smc.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/controllers/aspeed-smc.c b/drivers/mtd/spi-nor/controllers/aspeed-smc.c
index 7225870e8b18..acfe010f9dd7 100644
--- a/drivers/mtd/spi-nor/controllers/aspeed-smc.c
+++ b/drivers/mtd/spi-nor/controllers/aspeed-smc.c
@@ -769,6 +769,7 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
 	struct device_node *child;
 	unsigned int cs;
 	int ret = -ENODEV;
+	bool found_one = false;
 
 	for_each_available_child_of_node(np, child) {
 		struct aspeed_smc_chip *chip;
@@ -827,8 +828,17 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
 		 * by of property.
 		 */
 		ret = spi_nor_scan(nor, NULL, &hwcaps);
-		if (ret)
-			break;
+		/*
+		 * If we fail to scan the device it might not be present or
+		 * broken.  Don't fail the whole controller if others work.
+		 */
+		if (ret) {
+			if (found_one)
+				ret = 0;
+
+			devm_kfree(controller->dev, chip);
+			continue;
+		}
 
 		ret = aspeed_smc_chip_setup_finish(chip);
 		if (ret)
@@ -839,6 +849,7 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
 			break;
 
 		controller->chips[cs] = chip;
+		found_one = true;
 	}
 
 	if (ret) {
-- 
2.32.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH] mtd: aspeed-smc: improve probe resilience

[Patrick Williams]

The aspeed-smc can have multiple SPI devices attached to it in the
device tree.  If one of the devices is missing or failing the entire
probe will fail and all MTD devices under the controller will be
removed.  On OpenBMC this results in a kernel panic due to missing
rootfs:

[    0.538774] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
[    0.540471] aspeed-smc 1e620000.spi: w25q01jv-iq (131072 Kbytes)
[    0.540750] aspeed-smc 1e620000.spi: CE0 window [ 0x20000000 - 0x28000000 ] 128MB
[    0.540943] aspeed-smc 1e620000.spi: CE1 window [ 0x28000000 - 0x2c000000 ] 64MB
[    0.541143] aspeed-smc 1e620000.spi: read control register: 203b0041
[    0.581442] 5 fixed-partitions partitions found on MTD device bmc
[    0.581625] Creating 5 MTD partitions on "bmc":
[    0.581854] 0x000000000000-0x0000000e0000 : "u-boot"
[    0.584472] 0x0000000e0000-0x000000100000 : "u-boot-env"
[    0.586468] 0x000000100000-0x000000a00000 : "kernel"
[    0.588465] 0x000000a00000-0x000006000000 : "rofs"
[    0.590552] 0x000006000000-0x000008000000 : "rwfs"
[    0.592605] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
[    0.592801] aspeed-smc 1e620000.spi: unrecognized JEDEC id bytes: 00 00 00 00 00 00
[    0.593039] Deleting MTD partitions on "bmc":
[    0.593175] Deleting u-boot MTD partition
[    0.637929] Deleting u-boot-env MTD partition
[    0.829527] Deleting kernel MTD partition
[    0.856902] Freeing initrd memory: 1032K
[    0.866428] Deleting rofs MTD partition
[    0.906264] Deleting rwfs MTD partition
[    0.986628] aspeed-smc 1e620000.spi: Aspeed SMC probe failed -2
[    0.986929] aspeed-smc: probe of 1e620000.spi failed with error -2
...
[    2.936719] /dev/mtdblock: Can't open blockdev
mount: mounting /dev/mtdblock on run/initramfs/ro failed: No such file or directory
[    2.963030] MTD: Couldn't look up '/dev/mtdblock': -2
mount: mounting /dev/mtdblock on run/initramfs/rw failed: No such file or directory

Mounting read-write /dev/mtdblock filesystem failed.  Please fix and run
	mount /dev/mtdblock run/initramfs/rw -t jffs2 -o rw
or perform a factory reset with the clean-rwfs-filesystem option.
Fatal error, triggering kernel panic!
[    3.013047] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000100

Many BMC designs have two flash chips so that they can handle a hardware
failure of one of them.  If one chip failed, it doesn't do any good to
have redundancy if they all get removed anyhow.

Improve the resilience of the probe function to handle one of the
children being missing or failed.  Only in the case where all children
fail to probe should the controller be failed out.

Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
---
 drivers/mtd/spi-nor/controllers/aspeed-smc.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/controllers/aspeed-smc.c b/drivers/mtd/spi-nor/controllers/aspeed-smc.c
index 7225870e8b18..acfe010f9dd7 100644
--- a/drivers/mtd/spi-nor/controllers/aspeed-smc.c
+++ b/drivers/mtd/spi-nor/controllers/aspeed-smc.c
@@ -769,6 +769,7 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
 	struct device_node *child;
 	unsigned int cs;
 	int ret = -ENODEV;
+	bool found_one = false;
 
 	for_each_available_child_of_node(np, child) {
 		struct aspeed_smc_chip *chip;
@@ -827,8 +828,17 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
 		 * by of property.
 		 */
 		ret = spi_nor_scan(nor, NULL, &hwcaps);
-		if (ret)
-			break;
+		/*
+		 * If we fail to scan the device it might not be present or
+		 * broken.  Don't fail the whole controller if others work.
+		 */
+		if (ret) {
+			if (found_one)
+				ret = 0;
+
+			devm_kfree(controller->dev, chip);
+			continue;
+		}
 
 		ret = aspeed_smc_chip_setup_finish(chip);
 		if (ret)
@@ -839,6 +849,7 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
 			break;
 
 		controller->chips[cs] = chip;
+		found_one = true;
 	}
 
 	if (ret) {
-- 
2.32.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Pratyush Yadav]

Hi,

On 29/12/21 08:33AM, Patrick Williams wrote:
> The aspeed-smc can have multiple SPI devices attached to it in the
> device tree.  If one of the devices is missing or failing the entire
> probe will fail and all MTD devices under the controller will be
> removed.  On OpenBMC this results in a kernel panic due to missing
> rootfs:
> 
> [    0.538774] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
> [    0.540471] aspeed-smc 1e620000.spi: w25q01jv-iq (131072 Kbytes)
> [    0.540750] aspeed-smc 1e620000.spi: CE0 window [ 0x20000000 - 0x28000000 ] 128MB
> [    0.540943] aspeed-smc 1e620000.spi: CE1 window [ 0x28000000 - 0x2c000000 ] 64MB
> [    0.541143] aspeed-smc 1e620000.spi: read control register: 203b0041
> [    0.581442] 5 fixed-partitions partitions found on MTD device bmc
> [    0.581625] Creating 5 MTD partitions on "bmc":
> [    0.581854] 0x000000000000-0x0000000e0000 : "u-boot"
> [    0.584472] 0x0000000e0000-0x000000100000 : "u-boot-env"
> [    0.586468] 0x000000100000-0x000000a00000 : "kernel"
> [    0.588465] 0x000000a00000-0x000006000000 : "rofs"
> [    0.590552] 0x000006000000-0x000008000000 : "rwfs"
> [    0.592605] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
> [    0.592801] aspeed-smc 1e620000.spi: unrecognized JEDEC id bytes: 00 00 00 00 00 00
> [    0.593039] Deleting MTD partitions on "bmc":
> [    0.593175] Deleting u-boot MTD partition
> [    0.637929] Deleting u-boot-env MTD partition
> [    0.829527] Deleting kernel MTD partition
> [    0.856902] Freeing initrd memory: 1032K
> [    0.866428] Deleting rofs MTD partition
> [    0.906264] Deleting rwfs MTD partition
> [    0.986628] aspeed-smc 1e620000.spi: Aspeed SMC probe failed -2
> [    0.986929] aspeed-smc: probe of 1e620000.spi failed with error -2
> ...
> [    2.936719] /dev/mtdblock: Can't open blockdev
> mount: mounting /dev/mtdblock on run/initramfs/ro failed: No such file or directory
> [    2.963030] MTD: Couldn't look up '/dev/mtdblock': -2
> mount: mounting /dev/mtdblock on run/initramfs/rw failed: No such file or directory
> 
> Mounting read-write /dev/mtdblock filesystem failed.  Please fix and run
> 	mount /dev/mtdblock run/initramfs/rw -t jffs2 -o rw
> or perform a factory reset with the clean-rwfs-filesystem option.
> Fatal error, triggering kernel panic!
> [    3.013047] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000100
> 
> Many BMC designs have two flash chips so that they can handle a hardware
> failure of one of them.  If one chip failed, it doesn't do any good to
> have redundancy if they all get removed anyhow.
> 
> Improve the resilience of the probe function to handle one of the
> children being missing or failed.  Only in the case where all children
> fail to probe should the controller be failed out.

The patch itself looks fine to me but we no longer want to maintain 
drivers under drivers/mtd/spi-nor/controllers/. They should be moved to 
implement the SPI MEM API (under drivers/spi/). See [0][1] for a couple 
examples. Could you please volunteer to do the conversion for this 
driver?

[0] https://patchwork.ozlabs.org/project/linux-mtd/patch/20200601070444.16923-8-vigneshr@ti.com/
[1] https://patchwork.ozlabs.org/project/linux-mtd/patch/20211220164625.9400-3-mika.westerberg@linux.intel.com/

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Pratyush Yadav]

Hi,

On 29/12/21 08:33AM, Patrick Williams wrote:
> The aspeed-smc can have multiple SPI devices attached to it in the
> device tree.  If one of the devices is missing or failing the entire
> probe will fail and all MTD devices under the controller will be
> removed.  On OpenBMC this results in a kernel panic due to missing
> rootfs:
> 
> [    0.538774] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
> [    0.540471] aspeed-smc 1e620000.spi: w25q01jv-iq (131072 Kbytes)
> [    0.540750] aspeed-smc 1e620000.spi: CE0 window [ 0x20000000 - 0x28000000 ] 128MB
> [    0.540943] aspeed-smc 1e620000.spi: CE1 window [ 0x28000000 - 0x2c000000 ] 64MB
> [    0.541143] aspeed-smc 1e620000.spi: read control register: 203b0041
> [    0.581442] 5 fixed-partitions partitions found on MTD device bmc
> [    0.581625] Creating 5 MTD partitions on "bmc":
> [    0.581854] 0x000000000000-0x0000000e0000 : "u-boot"
> [    0.584472] 0x0000000e0000-0x000000100000 : "u-boot-env"
> [    0.586468] 0x000000100000-0x000000a00000 : "kernel"
> [    0.588465] 0x000000a00000-0x000006000000 : "rofs"
> [    0.590552] 0x000006000000-0x000008000000 : "rwfs"
> [    0.592605] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
> [    0.592801] aspeed-smc 1e620000.spi: unrecognized JEDEC id bytes: 00 00 00 00 00 00
> [    0.593039] Deleting MTD partitions on "bmc":
> [    0.593175] Deleting u-boot MTD partition
> [    0.637929] Deleting u-boot-env MTD partition
> [    0.829527] Deleting kernel MTD partition
> [    0.856902] Freeing initrd memory: 1032K
> [    0.866428] Deleting rofs MTD partition
> [    0.906264] Deleting rwfs MTD partition
> [    0.986628] aspeed-smc 1e620000.spi: Aspeed SMC probe failed -2
> [    0.986929] aspeed-smc: probe of 1e620000.spi failed with error -2
> ...
> [    2.936719] /dev/mtdblock: Can't open blockdev
> mount: mounting /dev/mtdblock on run/initramfs/ro failed: No such file or directory
> [    2.963030] MTD: Couldn't look up '/dev/mtdblock': -2
> mount: mounting /dev/mtdblock on run/initramfs/rw failed: No such file or directory
> 
> Mounting read-write /dev/mtdblock filesystem failed.  Please fix and run
> 	mount /dev/mtdblock run/initramfs/rw -t jffs2 -o rw
> or perform a factory reset with the clean-rwfs-filesystem option.
> Fatal error, triggering kernel panic!
> [    3.013047] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000100
> 
> Many BMC designs have two flash chips so that they can handle a hardware
> failure of one of them.  If one chip failed, it doesn't do any good to
> have redundancy if they all get removed anyhow.
> 
> Improve the resilience of the probe function to handle one of the
> children being missing or failed.  Only in the case where all children
> fail to probe should the controller be failed out.

The patch itself looks fine to me but we no longer want to maintain 
drivers under drivers/mtd/spi-nor/controllers/. They should be moved to 
implement the SPI MEM API (under drivers/spi/). See [0][1] for a couple 
examples. Could you please volunteer to do the conversion for this 
driver?

[0] https://patchwork.ozlabs.org/project/linux-mtd/patch/20200601070444.16923-8-vigneshr@ti.com/
[1] https://patchwork.ozlabs.org/project/linux-mtd/patch/20211220164625.9400-3-mika.westerberg@linux.intel.com/

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Pratyush Yadav]

Hi,

On 29/12/21 08:33AM, Patrick Williams wrote:
> The aspeed-smc can have multiple SPI devices attached to it in the
> device tree.  If one of the devices is missing or failing the entire
> probe will fail and all MTD devices under the controller will be
> removed.  On OpenBMC this results in a kernel panic due to missing
> rootfs:
> 
> [    0.538774] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
> [    0.540471] aspeed-smc 1e620000.spi: w25q01jv-iq (131072 Kbytes)
> [    0.540750] aspeed-smc 1e620000.spi: CE0 window [ 0x20000000 - 0x28000000 ] 128MB
> [    0.540943] aspeed-smc 1e620000.spi: CE1 window [ 0x28000000 - 0x2c000000 ] 64MB
> [    0.541143] aspeed-smc 1e620000.spi: read control register: 203b0041
> [    0.581442] 5 fixed-partitions partitions found on MTD device bmc
> [    0.581625] Creating 5 MTD partitions on "bmc":
> [    0.581854] 0x000000000000-0x0000000e0000 : "u-boot"
> [    0.584472] 0x0000000e0000-0x000000100000 : "u-boot-env"
> [    0.586468] 0x000000100000-0x000000a00000 : "kernel"
> [    0.588465] 0x000000a00000-0x000006000000 : "rofs"
> [    0.590552] 0x000006000000-0x000008000000 : "rwfs"
> [    0.592605] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
> [    0.592801] aspeed-smc 1e620000.spi: unrecognized JEDEC id bytes: 00 00 00 00 00 00
> [    0.593039] Deleting MTD partitions on "bmc":
> [    0.593175] Deleting u-boot MTD partition
> [    0.637929] Deleting u-boot-env MTD partition
> [    0.829527] Deleting kernel MTD partition
> [    0.856902] Freeing initrd memory: 1032K
> [    0.866428] Deleting rofs MTD partition
> [    0.906264] Deleting rwfs MTD partition
> [    0.986628] aspeed-smc 1e620000.spi: Aspeed SMC probe failed -2
> [    0.986929] aspeed-smc: probe of 1e620000.spi failed with error -2
> ...
> [    2.936719] /dev/mtdblock: Can't open blockdev
> mount: mounting /dev/mtdblock on run/initramfs/ro failed: No such file or directory
> [    2.963030] MTD: Couldn't look up '/dev/mtdblock': -2
> mount: mounting /dev/mtdblock on run/initramfs/rw failed: No such file or directory
> 
> Mounting read-write /dev/mtdblock filesystem failed.  Please fix and run
> 	mount /dev/mtdblock run/initramfs/rw -t jffs2 -o rw
> or perform a factory reset with the clean-rwfs-filesystem option.
> Fatal error, triggering kernel panic!
> [    3.013047] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000100
> 
> Many BMC designs have two flash chips so that they can handle a hardware
> failure of one of them.  If one chip failed, it doesn't do any good to
> have redundancy if they all get removed anyhow.
> 
> Improve the resilience of the probe function to handle one of the
> children being missing or failed.  Only in the case where all children
> fail to probe should the controller be failed out.

The patch itself looks fine to me but we no longer want to maintain 
drivers under drivers/mtd/spi-nor/controllers/. They should be moved to 
implement the SPI MEM API (under drivers/spi/). See [0][1] for a couple 
examples. Could you please volunteer to do the conversion for this 
driver?

[0] https://patchwork.ozlabs.org/project/linux-mtd/patch/20200601070444.16923-8-vigneshr@ti.com/
[1] https://patchwork.ozlabs.org/project/linux-mtd/patch/20211220164625.9400-3-mika.westerberg@linux.intel.com/

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Patrick Williams]

[-- Attachment #1: Type: text/plain, Size: 1194 bytes --]

On Wed, Dec 29, 2021 at 11:04:13PM +0530, Pratyush Yadav wrote:
> Hi,
> 
> On 29/12/21 08:33AM, Patrick Williams wrote:
 
> The patch itself looks fine to me but we no longer want to maintain 
> drivers under drivers/mtd/spi-nor/controllers/. They should be moved to 
> implement the SPI MEM API (under drivers/spi/).

Is the linux-aspeed community aware of this?  Are you saying you don't want to
take fixes for their driver into the MTD tree?  Can it be pulled into the Aspeed
tree?

> Could you please volunteer to do the conversion for this driver?

I'm not personally going to be able to get to it for at least the next 3 months.

It looks like we don't have a dedicated maintainer for this driver other than
Joel by nature of him being listed as the maintainer of "ARM/ASPEED MACHINE
SUPPORT".  I'm not sure if Aspeed themselves are planning on doing the necessary
refactoring here.


Joel, are you aware of this driver using a deprecated implementation?  Were
there anyone planning to do the reworks that you are aware of?  I'd like to get
this fix at least into the OpenBMC kernel tree because I'm seeing this fail in
the real world.

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Patrick Williams]

[-- Attachment #1.1: Type: text/plain, Size: 1194 bytes --]

On Wed, Dec 29, 2021 at 11:04:13PM +0530, Pratyush Yadav wrote:
> Hi,
> 
> On 29/12/21 08:33AM, Patrick Williams wrote:
 
> The patch itself looks fine to me but we no longer want to maintain 
> drivers under drivers/mtd/spi-nor/controllers/. They should be moved to 
> implement the SPI MEM API (under drivers/spi/).

Is the linux-aspeed community aware of this?  Are you saying you don't want to
take fixes for their driver into the MTD tree?  Can it be pulled into the Aspeed
tree?

> Could you please volunteer to do the conversion for this driver?

I'm not personally going to be able to get to it for at least the next 3 months.

It looks like we don't have a dedicated maintainer for this driver other than
Joel by nature of him being listed as the maintainer of "ARM/ASPEED MACHINE
SUPPORT".  I'm not sure if Aspeed themselves are planning on doing the necessary
refactoring here.


Joel, are you aware of this driver using a deprecated implementation?  Were
there anyone planning to do the reworks that you are aware of?  I'd like to get
this fix at least into the OpenBMC kernel tree because I'm seeing this fail in
the real world.

-- 
Patrick Williams

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Patrick Williams]

[-- Attachment #1.1: Type: text/plain, Size: 1194 bytes --]

On Wed, Dec 29, 2021 at 11:04:13PM +0530, Pratyush Yadav wrote:
> Hi,
> 
> On 29/12/21 08:33AM, Patrick Williams wrote:
 
> The patch itself looks fine to me but we no longer want to maintain 
> drivers under drivers/mtd/spi-nor/controllers/. They should be moved to 
> implement the SPI MEM API (under drivers/spi/).

Is the linux-aspeed community aware of this?  Are you saying you don't want to
take fixes for their driver into the MTD tree?  Can it be pulled into the Aspeed
tree?

> Could you please volunteer to do the conversion for this driver?

I'm not personally going to be able to get to it for at least the next 3 months.

It looks like we don't have a dedicated maintainer for this driver other than
Joel by nature of him being listed as the maintainer of "ARM/ASPEED MACHINE
SUPPORT".  I'm not sure if Aspeed themselves are planning on doing the necessary
refactoring here.


Joel, are you aware of this driver using a deprecated implementation?  Were
there anyone planning to do the reworks that you are aware of?  I'd like to get
this fix at least into the OpenBMC kernel tree because I'm seeing this fail in
the real world.

-- 
Patrick Williams

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Pratyush Yadav]

Hi Patrick,

On 30/12/21 09:29AM, Patrick Williams wrote:
> On Wed, Dec 29, 2021 at 11:04:13PM +0530, Pratyush Yadav wrote:
> > Hi,
> > 
> > On 29/12/21 08:33AM, Patrick Williams wrote:
>  
> > The patch itself looks fine to me but we no longer want to maintain 
> > drivers under drivers/mtd/spi-nor/controllers/. They should be moved to 
> > implement the SPI MEM API (under drivers/spi/).
> 
> Is the linux-aspeed community aware of this?  Are you saying you don't want to
> take fixes for their driver into the MTD tree?  Can it be pulled into the Aspeed
> tree?

I am fine with taking in bug fixes but no longer want to take in any new 
features. My main intention was to nudge you to convert it to SPI MEM 
regardless of whether this is a bug fix or a new feature, because 
eventually we want to get rid of drivers/mtd/spi-nor/controllers/ and 
the API that comes along with it.

As for your patch, I certainly don't want it to go via the aspeed tree. 
It should go via the MTD tree or not at all. I am not quite sure if this 
counts as a bug fix or a new feature though.

> 
> > Could you please volunteer to do the conversion for this driver?
> 
> I'm not personally going to be able to get to it for at least the next 3 months.
> 
> It looks like we don't have a dedicated maintainer for this driver other than
> Joel by nature of him being listed as the maintainer of "ARM/ASPEED MACHINE
> SUPPORT".  I'm not sure if Aspeed themselves are planning on doing the necessary
> refactoring here.
> 
> 
> Joel, are you aware of this driver using a deprecated implementation?  Were
> there anyone planning to do the reworks that you are aware of?  I'd like to get
> this fix at least into the OpenBMC kernel tree because I'm seeing this fail in
> the real world.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Pratyush Yadav]

Hi Patrick,

On 30/12/21 09:29AM, Patrick Williams wrote:
> On Wed, Dec 29, 2021 at 11:04:13PM +0530, Pratyush Yadav wrote:
> > Hi,
> > 
> > On 29/12/21 08:33AM, Patrick Williams wrote:
>  
> > The patch itself looks fine to me but we no longer want to maintain 
> > drivers under drivers/mtd/spi-nor/controllers/. They should be moved to 
> > implement the SPI MEM API (under drivers/spi/).
> 
> Is the linux-aspeed community aware of this?  Are you saying you don't want to
> take fixes for their driver into the MTD tree?  Can it be pulled into the Aspeed
> tree?

I am fine with taking in bug fixes but no longer want to take in any new 
features. My main intention was to nudge you to convert it to SPI MEM 
regardless of whether this is a bug fix or a new feature, because 
eventually we want to get rid of drivers/mtd/spi-nor/controllers/ and 
the API that comes along with it.

As for your patch, I certainly don't want it to go via the aspeed tree. 
It should go via the MTD tree or not at all. I am not quite sure if this 
counts as a bug fix or a new feature though.

> 
> > Could you please volunteer to do the conversion for this driver?
> 
> I'm not personally going to be able to get to it for at least the next 3 months.
> 
> It looks like we don't have a dedicated maintainer for this driver other than
> Joel by nature of him being listed as the maintainer of "ARM/ASPEED MACHINE
> SUPPORT".  I'm not sure if Aspeed themselves are planning on doing the necessary
> refactoring here.
> 
> 
> Joel, are you aware of this driver using a deprecated implementation?  Were
> there anyone planning to do the reworks that you are aware of?  I'd like to get
> this fix at least into the OpenBMC kernel tree because I'm seeing this fail in
> the real world.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Pratyush Yadav]

Hi Patrick,

On 30/12/21 09:29AM, Patrick Williams wrote:
> On Wed, Dec 29, 2021 at 11:04:13PM +0530, Pratyush Yadav wrote:
> > Hi,
> > 
> > On 29/12/21 08:33AM, Patrick Williams wrote:
>  
> > The patch itself looks fine to me but we no longer want to maintain 
> > drivers under drivers/mtd/spi-nor/controllers/. They should be moved to 
> > implement the SPI MEM API (under drivers/spi/).
> 
> Is the linux-aspeed community aware of this?  Are you saying you don't want to
> take fixes for their driver into the MTD tree?  Can it be pulled into the Aspeed
> tree?

I am fine with taking in bug fixes but no longer want to take in any new 
features. My main intention was to nudge you to convert it to SPI MEM 
regardless of whether this is a bug fix or a new feature, because 
eventually we want to get rid of drivers/mtd/spi-nor/controllers/ and 
the API that comes along with it.

As for your patch, I certainly don't want it to go via the aspeed tree. 
It should go via the MTD tree or not at all. I am not quite sure if this 
counts as a bug fix or a new feature though.

> 
> > Could you please volunteer to do the conversion for this driver?
> 
> I'm not personally going to be able to get to it for at least the next 3 months.
> 
> It looks like we don't have a dedicated maintainer for this driver other than
> Joel by nature of him being listed as the maintainer of "ARM/ASPEED MACHINE
> SUPPORT".  I'm not sure if Aspeed themselves are planning on doing the necessary
> refactoring here.
> 
> 
> Joel, are you aware of this driver using a deprecated implementation?  Were
> there anyone planning to do the reworks that you are aware of?  I'd like to get
> this fix at least into the OpenBMC kernel tree because I'm seeing this fail in
> the real world.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Miquel Raynal]

Hello,

p.yadav@ti.com wrote on Fri, 31 Dec 2021 15:56:25 +0530:

> Hi Patrick,
> 
> On 30/12/21 09:29AM, Patrick Williams wrote:
> > On Wed, Dec 29, 2021 at 11:04:13PM +0530, Pratyush Yadav wrote:  
> > > Hi,
> > > 
> > > On 29/12/21 08:33AM, Patrick Williams wrote:  
> >    
> > > The patch itself looks fine to me but we no longer want to maintain 
> > > drivers under drivers/mtd/spi-nor/controllers/. They should be moved to 
> > > implement the SPI MEM API (under drivers/spi/).  
> > 
> > Is the linux-aspeed community aware of this?  Are you saying you don't want to
> > take fixes for their driver into the MTD tree?  Can it be pulled into the Aspeed
> > tree?  
> 
> I am fine with taking in bug fixes but no longer want to take in any new 
> features. My main intention was to nudge you to convert it to SPI MEM 
> regardless of whether this is a bug fix or a new feature, because 
> eventually we want to get rid of drivers/mtd/spi-nor/controllers/ and 
> the API that comes along with it.

I totally agree with Pratyush here, we no longer want to support the
spi-nor controller API so if, as you say, there are boards failing
in the field, it means there are still users and these users must be
warned that at some point we might discontinue the support of these
drivers if it becomes too painful.

> As for your patch, I certainly don't want it to go via the aspeed tree.

Definitely, no.

> It should go via the MTD tree or not at all. I am not quite sure if this 
> counts as a bug fix or a new feature though.
> 
> >   
> > > Could you please volunteer to do the conversion for this driver?  
> > 
> > I'm not personally going to be able to get to it for at least the next 3 months.
> > 
> > It looks like we don't have a dedicated maintainer for this driver other than
> > Joel by nature of him being listed as the maintainer of "ARM/ASPEED MACHINE
> > SUPPORT".  I'm not sure if Aspeed themselves are planning on doing the necessary
> > refactoring here.
> > 
> > 
> > Joel, are you aware of this driver using a deprecated implementation?  Were
> > there anyone planning to do the reworks that you are aware of?  I'd like to get
> > this fix at least into the OpenBMC kernel tree because I'm seeing this fail in
> > the real world.  
> 

Thanks,
Miquèl

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Miquel Raynal]

Hello,

p.yadav@ti.com wrote on Fri, 31 Dec 2021 15:56:25 +0530:

> Hi Patrick,
> 
> On 30/12/21 09:29AM, Patrick Williams wrote:
> > On Wed, Dec 29, 2021 at 11:04:13PM +0530, Pratyush Yadav wrote:  
> > > Hi,
> > > 
> > > On 29/12/21 08:33AM, Patrick Williams wrote:  
> >    
> > > The patch itself looks fine to me but we no longer want to maintain 
> > > drivers under drivers/mtd/spi-nor/controllers/. They should be moved to 
> > > implement the SPI MEM API (under drivers/spi/).  
> > 
> > Is the linux-aspeed community aware of this?  Are you saying you don't want to
> > take fixes for their driver into the MTD tree?  Can it be pulled into the Aspeed
> > tree?  
> 
> I am fine with taking in bug fixes but no longer want to take in any new 
> features. My main intention was to nudge you to convert it to SPI MEM 
> regardless of whether this is a bug fix or a new feature, because 
> eventually we want to get rid of drivers/mtd/spi-nor/controllers/ and 
> the API that comes along with it.

I totally agree with Pratyush here, we no longer want to support the
spi-nor controller API so if, as you say, there are boards failing
in the field, it means there are still users and these users must be
warned that at some point we might discontinue the support of these
drivers if it becomes too painful.

> As for your patch, I certainly don't want it to go via the aspeed tree.

Definitely, no.

> It should go via the MTD tree or not at all. I am not quite sure if this 
> counts as a bug fix or a new feature though.
> 
> >   
> > > Could you please volunteer to do the conversion for this driver?  
> > 
> > I'm not personally going to be able to get to it for at least the next 3 months.
> > 
> > It looks like we don't have a dedicated maintainer for this driver other than
> > Joel by nature of him being listed as the maintainer of "ARM/ASPEED MACHINE
> > SUPPORT".  I'm not sure if Aspeed themselves are planning on doing the necessary
> > refactoring here.
> > 
> > 
> > Joel, are you aware of this driver using a deprecated implementation?  Were
> > there anyone planning to do the reworks that you are aware of?  I'd like to get
> > this fix at least into the OpenBMC kernel tree because I'm seeing this fail in
> > the real world.  
> 

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Miquel Raynal]

Hello,

p.yadav@ti.com wrote on Fri, 31 Dec 2021 15:56:25 +0530:

> Hi Patrick,
> 
> On 30/12/21 09:29AM, Patrick Williams wrote:
> > On Wed, Dec 29, 2021 at 11:04:13PM +0530, Pratyush Yadav wrote:  
> > > Hi,
> > > 
> > > On 29/12/21 08:33AM, Patrick Williams wrote:  
> >    
> > > The patch itself looks fine to me but we no longer want to maintain 
> > > drivers under drivers/mtd/spi-nor/controllers/. They should be moved to 
> > > implement the SPI MEM API (under drivers/spi/).  
> > 
> > Is the linux-aspeed community aware of this?  Are you saying you don't want to
> > take fixes for their driver into the MTD tree?  Can it be pulled into the Aspeed
> > tree?  
> 
> I am fine with taking in bug fixes but no longer want to take in any new 
> features. My main intention was to nudge you to convert it to SPI MEM 
> regardless of whether this is a bug fix or a new feature, because 
> eventually we want to get rid of drivers/mtd/spi-nor/controllers/ and 
> the API that comes along with it.

I totally agree with Pratyush here, we no longer want to support the
spi-nor controller API so if, as you say, there are boards failing
in the field, it means there are still users and these users must be
warned that at some point we might discontinue the support of these
drivers if it becomes too painful.

> As for your patch, I certainly don't want it to go via the aspeed tree.

Definitely, no.

> It should go via the MTD tree or not at all. I am not quite sure if this 
> counts as a bug fix or a new feature though.
> 
> >   
> > > Could you please volunteer to do the conversion for this driver?  
> > 
> > I'm not personally going to be able to get to it for at least the next 3 months.
> > 
> > It looks like we don't have a dedicated maintainer for this driver other than
> > Joel by nature of him being listed as the maintainer of "ARM/ASPEED MACHINE
> > SUPPORT".  I'm not sure if Aspeed themselves are planning on doing the necessary
> > refactoring here.
> > 
> > 
> > Joel, are you aware of this driver using a deprecated implementation?  Were
> > there anyone planning to do the reworks that you are aware of?  I'd like to get
> > this fix at least into the OpenBMC kernel tree because I'm seeing this fail in
> > the real world.  
> 

Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Patrick Williams]

[-- Attachment #1: Type: text/plain, Size: 2238 bytes --]

Hi Miquel,

On Mon, Jan 03, 2022 at 05:17:21PM +0100, Miquel Raynal wrote:

> > I am fine with taking in bug fixes but no longer want to take in any new 
> > features. My main intention was to nudge you to convert it to SPI MEM 
> > regardless of whether this is a bug fix or a new feature, because 
> > eventually we want to get rid of drivers/mtd/spi-nor/controllers/ and 
> > the API that comes along with it.
> 
> I totally agree with Pratyush here, we no longer want to support the
> spi-nor controller API so if, as you say, there are boards failing
> in the field, it means there are still users and these users must be
> warned that at some point we might discontinue the support of these
> drivers if it becomes too painful.
>

Your response here makes it seem like you don't realize the scope of this
driver.  There are probably, by my estimates, on the order of 10s of millions of
deployed systems using this driver in the world.  The vast majority of servers
in the world use an AST2400, AST2500, or AST2600 chip, which needs this driver
in order access its own flash storage device.  Both OpenBMC and most of the
proprietary alternatives use this driver.

The company I work for has a LOT of systems using this code.  After I made this
fix, for a new design being developed, it was pointed out to me that our ODM ran
into this problem a few years ago and we've been really bad about upstreaming
those fixes.  For this new system I'm trying to keep us on top of all
upstreaming efforts.

To me the inability to access your own storage, resulting in a kernel panic, is
a pretty serious issue.  Bug or feature I guess is always in the eye of the
beholder though.  I think this is pretty valuable to get in, from an impact
standpoint, and pretty minimal in terms of maintenance efforts on the
maintainers part.

I had an offline discussion with someone who knew more history on this driver.
My understanding is that the linux-aspeed team is aware of this being deprecated
but that there was some missing support for interface training that nobody has
gotten around to write?  If that is the case this really isn't even a "simple"
port to a new API at this point.

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Patrick Williams]

[-- Attachment #1.1: Type: text/plain, Size: 2238 bytes --]

Hi Miquel,

On Mon, Jan 03, 2022 at 05:17:21PM +0100, Miquel Raynal wrote:

> > I am fine with taking in bug fixes but no longer want to take in any new 
> > features. My main intention was to nudge you to convert it to SPI MEM 
> > regardless of whether this is a bug fix or a new feature, because 
> > eventually we want to get rid of drivers/mtd/spi-nor/controllers/ and 
> > the API that comes along with it.
> 
> I totally agree with Pratyush here, we no longer want to support the
> spi-nor controller API so if, as you say, there are boards failing
> in the field, it means there are still users and these users must be
> warned that at some point we might discontinue the support of these
> drivers if it becomes too painful.
>

Your response here makes it seem like you don't realize the scope of this
driver.  There are probably, by my estimates, on the order of 10s of millions of
deployed systems using this driver in the world.  The vast majority of servers
in the world use an AST2400, AST2500, or AST2600 chip, which needs this driver
in order access its own flash storage device.  Both OpenBMC and most of the
proprietary alternatives use this driver.

The company I work for has a LOT of systems using this code.  After I made this
fix, for a new design being developed, it was pointed out to me that our ODM ran
into this problem a few years ago and we've been really bad about upstreaming
those fixes.  For this new system I'm trying to keep us on top of all
upstreaming efforts.

To me the inability to access your own storage, resulting in a kernel panic, is
a pretty serious issue.  Bug or feature I guess is always in the eye of the
beholder though.  I think this is pretty valuable to get in, from an impact
standpoint, and pretty minimal in terms of maintenance efforts on the
maintainers part.

I had an offline discussion with someone who knew more history on this driver.
My understanding is that the linux-aspeed team is aware of this being deprecated
but that there was some missing support for interface training that nobody has
gotten around to write?  If that is the case this really isn't even a "simple"
port to a new API at this point.

-- 
Patrick Williams

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Patrick Williams]

[-- Attachment #1.1: Type: text/plain, Size: 2238 bytes --]

Hi Miquel,

On Mon, Jan 03, 2022 at 05:17:21PM +0100, Miquel Raynal wrote:

> > I am fine with taking in bug fixes but no longer want to take in any new 
> > features. My main intention was to nudge you to convert it to SPI MEM 
> > regardless of whether this is a bug fix or a new feature, because 
> > eventually we want to get rid of drivers/mtd/spi-nor/controllers/ and 
> > the API that comes along with it.
> 
> I totally agree with Pratyush here, we no longer want to support the
> spi-nor controller API so if, as you say, there are boards failing
> in the field, it means there are still users and these users must be
> warned that at some point we might discontinue the support of these
> drivers if it becomes too painful.
>

Your response here makes it seem like you don't realize the scope of this
driver.  There are probably, by my estimates, on the order of 10s of millions of
deployed systems using this driver in the world.  The vast majority of servers
in the world use an AST2400, AST2500, or AST2600 chip, which needs this driver
in order access its own flash storage device.  Both OpenBMC and most of the
proprietary alternatives use this driver.

The company I work for has a LOT of systems using this code.  After I made this
fix, for a new design being developed, it was pointed out to me that our ODM ran
into this problem a few years ago and we've been really bad about upstreaming
those fixes.  For this new system I'm trying to keep us on top of all
upstreaming efforts.

To me the inability to access your own storage, resulting in a kernel panic, is
a pretty serious issue.  Bug or feature I guess is always in the eye of the
beholder though.  I think this is pretty valuable to get in, from an impact
standpoint, and pretty minimal in terms of maintenance efforts on the
maintainers part.

I had an offline discussion with someone who knew more history on this driver.
My understanding is that the linux-aspeed team is aware of this being deprecated
but that there was some missing support for interface training that nobody has
gotten around to write?  If that is the case this really isn't even a "simple"
port to a new API at this point.

-- 
Patrick Williams

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Pratyush Yadav]

On 04/01/22 12:20PM, Patrick Williams wrote:
> Hi Miquel,
> 
> On Mon, Jan 03, 2022 at 05:17:21PM +0100, Miquel Raynal wrote:
> 
> > > I am fine with taking in bug fixes but no longer want to take in any new 
> > > features. My main intention was to nudge you to convert it to SPI MEM 
> > > regardless of whether this is a bug fix or a new feature, because 
> > > eventually we want to get rid of drivers/mtd/spi-nor/controllers/ and 
> > > the API that comes along with it.
> > 
> > I totally agree with Pratyush here, we no longer want to support the
> > spi-nor controller API so if, as you say, there are boards failing
> > in the field, it means there are still users and these users must be
> > warned that at some point we might discontinue the support of these
> > drivers if it becomes too painful.
> >
> 
> Your response here makes it seem like you don't realize the scope of this
> driver.  There are probably, by my estimates, on the order of 10s of millions of
> deployed systems using this driver in the world.  The vast majority of servers
> in the world use an AST2400, AST2500, or AST2600 chip, which needs this driver
> in order access its own flash storage device.  Both OpenBMC and most of the
> proprietary alternatives use this driver.

Then we should easily be able to find people willing to spend a couple 
days on updating the driver :-)

> 
> The company I work for has a LOT of systems using this code.  After I made this
> fix, for a new design being developed, it was pointed out to me that our ODM ran
> into this problem a few years ago and we've been really bad about upstreaming
> those fixes.  For this new system I'm trying to keep us on top of all
> upstreaming efforts.

If a company wants to use an upstream kernel for their systems I think 
they should invest into maintaining the drivers they are using.

> 
> To me the inability to access your own storage, resulting in a kernel panic, is
> a pretty serious issue.  Bug or feature I guess is always in the eye of the

One option you always have is to disable the bad flash in your device 
tree. I don't see why you would want to keep a flash that does not work 
enabled anyway.

> beholder though.  I think this is pretty valuable to get in, from an impact
> standpoint, and pretty minimal in terms of maintenance efforts on the
> maintainers part.

Yes, I agree that your patch itself has fairly low maintenance burden. I 
would not be too opposed to taking it in. But for the driver as a whole, 
that is indeed a maintenance burden since we have to carry code in SPI 
NOR to support it which makes adding new features a bit tricky.

We had a discussion along these lines for old unmaintained flashes in 
SPI NOR. The idea then was to warn the people who touched code related 
to those flashes that they can either update it or we will drop the 
flashes after X releases. They would still work on older kernels of 
course, but if there are any upstream users they would have to update 
the code or live without the flashes.

I would like to use the same approach for the controllers API as well. 
We can't keep carrying around legacy code forever just because a 
device/driver has no active developers. If people want to use some 
driver in the upstream kernel, they should help maintain it.

> 
> I had an offline discussion with someone who knew more history on this driver.
> My understanding is that the linux-aspeed team is aware of this being deprecated
> but that there was some missing support for interface training that nobody has
> gotten around to write?  If that is the case this really isn't even a "simple"
> port to a new API at this point.

Unless the controller needs some unique feature (I don't think it does 
on a quick glance), the conversion should not be too difficult. For any 
experienced developer, even if they are unfamiliar with the SPI MEM API, 
I don't think it should take more than 2-3 days to do the conversion. 
The code to program the registers would stay the same, all that needs to 
change is the API through which it is accessed.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Pratyush Yadav]

On 04/01/22 12:20PM, Patrick Williams wrote:
> Hi Miquel,
> 
> On Mon, Jan 03, 2022 at 05:17:21PM +0100, Miquel Raynal wrote:
> 
> > > I am fine with taking in bug fixes but no longer want to take in any new 
> > > features. My main intention was to nudge you to convert it to SPI MEM 
> > > regardless of whether this is a bug fix or a new feature, because 
> > > eventually we want to get rid of drivers/mtd/spi-nor/controllers/ and 
> > > the API that comes along with it.
> > 
> > I totally agree with Pratyush here, we no longer want to support the
> > spi-nor controller API so if, as you say, there are boards failing
> > in the field, it means there are still users and these users must be
> > warned that at some point we might discontinue the support of these
> > drivers if it becomes too painful.
> >
> 
> Your response here makes it seem like you don't realize the scope of this
> driver.  There are probably, by my estimates, on the order of 10s of millions of
> deployed systems using this driver in the world.  The vast majority of servers
> in the world use an AST2400, AST2500, or AST2600 chip, which needs this driver
> in order access its own flash storage device.  Both OpenBMC and most of the
> proprietary alternatives use this driver.

Then we should easily be able to find people willing to spend a couple 
days on updating the driver :-)

> 
> The company I work for has a LOT of systems using this code.  After I made this
> fix, for a new design being developed, it was pointed out to me that our ODM ran
> into this problem a few years ago and we've been really bad about upstreaming
> those fixes.  For this new system I'm trying to keep us on top of all
> upstreaming efforts.

If a company wants to use an upstream kernel for their systems I think 
they should invest into maintaining the drivers they are using.

> 
> To me the inability to access your own storage, resulting in a kernel panic, is
> a pretty serious issue.  Bug or feature I guess is always in the eye of the

One option you always have is to disable the bad flash in your device 
tree. I don't see why you would want to keep a flash that does not work 
enabled anyway.

> beholder though.  I think this is pretty valuable to get in, from an impact
> standpoint, and pretty minimal in terms of maintenance efforts on the
> maintainers part.

Yes, I agree that your patch itself has fairly low maintenance burden. I 
would not be too opposed to taking it in. But for the driver as a whole, 
that is indeed a maintenance burden since we have to carry code in SPI 
NOR to support it which makes adding new features a bit tricky.

We had a discussion along these lines for old unmaintained flashes in 
SPI NOR. The idea then was to warn the people who touched code related 
to those flashes that they can either update it or we will drop the 
flashes after X releases. They would still work on older kernels of 
course, but if there are any upstream users they would have to update 
the code or live without the flashes.

I would like to use the same approach for the controllers API as well. 
We can't keep carrying around legacy code forever just because a 
device/driver has no active developers. If people want to use some 
driver in the upstream kernel, they should help maintain it.

> 
> I had an offline discussion with someone who knew more history on this driver.
> My understanding is that the linux-aspeed team is aware of this being deprecated
> but that there was some missing support for interface training that nobody has
> gotten around to write?  If that is the case this really isn't even a "simple"
> port to a new API at this point.

Unless the controller needs some unique feature (I don't think it does 
on a quick glance), the conversion should not be too difficult. For any 
experienced developer, even if they are unfamiliar with the SPI MEM API, 
I don't think it should take more than 2-3 days to do the conversion. 
The code to program the registers would stay the same, all that needs to 
change is the API through which it is accessed.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Pratyush Yadav]

On 04/01/22 12:20PM, Patrick Williams wrote:
> Hi Miquel,
> 
> On Mon, Jan 03, 2022 at 05:17:21PM +0100, Miquel Raynal wrote:
> 
> > > I am fine with taking in bug fixes but no longer want to take in any new 
> > > features. My main intention was to nudge you to convert it to SPI MEM 
> > > regardless of whether this is a bug fix or a new feature, because 
> > > eventually we want to get rid of drivers/mtd/spi-nor/controllers/ and 
> > > the API that comes along with it.
> > 
> > I totally agree with Pratyush here, we no longer want to support the
> > spi-nor controller API so if, as you say, there are boards failing
> > in the field, it means there are still users and these users must be
> > warned that at some point we might discontinue the support of these
> > drivers if it becomes too painful.
> >
> 
> Your response here makes it seem like you don't realize the scope of this
> driver.  There are probably, by my estimates, on the order of 10s of millions of
> deployed systems using this driver in the world.  The vast majority of servers
> in the world use an AST2400, AST2500, or AST2600 chip, which needs this driver
> in order access its own flash storage device.  Both OpenBMC and most of the
> proprietary alternatives use this driver.

Then we should easily be able to find people willing to spend a couple 
days on updating the driver :-)

> 
> The company I work for has a LOT of systems using this code.  After I made this
> fix, for a new design being developed, it was pointed out to me that our ODM ran
> into this problem a few years ago and we've been really bad about upstreaming
> those fixes.  For this new system I'm trying to keep us on top of all
> upstreaming efforts.

If a company wants to use an upstream kernel for their systems I think 
they should invest into maintaining the drivers they are using.

> 
> To me the inability to access your own storage, resulting in a kernel panic, is
> a pretty serious issue.  Bug or feature I guess is always in the eye of the

One option you always have is to disable the bad flash in your device 
tree. I don't see why you would want to keep a flash that does not work 
enabled anyway.

> beholder though.  I think this is pretty valuable to get in, from an impact
> standpoint, and pretty minimal in terms of maintenance efforts on the
> maintainers part.

Yes, I agree that your patch itself has fairly low maintenance burden. I 
would not be too opposed to taking it in. But for the driver as a whole, 
that is indeed a maintenance burden since we have to carry code in SPI 
NOR to support it which makes adding new features a bit tricky.

We had a discussion along these lines for old unmaintained flashes in 
SPI NOR. The idea then was to warn the people who touched code related 
to those flashes that they can either update it or we will drop the 
flashes after X releases. They would still work on older kernels of 
course, but if there are any upstream users they would have to update 
the code or live without the flashes.

I would like to use the same approach for the controllers API as well. 
We can't keep carrying around legacy code forever just because a 
device/driver has no active developers. If people want to use some 
driver in the upstream kernel, they should help maintain it.

> 
> I had an offline discussion with someone who knew more history on this driver.
> My understanding is that the linux-aspeed team is aware of this being deprecated
> but that there was some missing support for interface training that nobody has
> gotten around to write?  If that is the case this really isn't even a "simple"
> port to a new API at this point.

Unless the controller needs some unique feature (I don't think it does 
on a quick glance), the conversion should not be too difficult. For any 
experienced developer, even if they are unfamiliar with the SPI MEM API, 
I don't think it should take more than 2-3 days to do the conversion. 
The code to program the registers would stay the same, all that needs to 
change is the API through which it is accessed.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Miquel Raynal]

On Wed, 2021-12-29 at 14:33:33 UTC, Patrick Williams wrote:
> The aspeed-smc can have multiple SPI devices attached to it in the
> device tree.  If one of the devices is missing or failing the entire
> probe will fail and all MTD devices under the controller will be
> removed.  On OpenBMC this results in a kernel panic due to missing
> rootfs:
> 
> [    0.538774] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
> [    0.540471] aspeed-smc 1e620000.spi: w25q01jv-iq (131072 Kbytes)
> [    0.540750] aspeed-smc 1e620000.spi: CE0 window [ 0x20000000 - 0x28000000 ] 128MB
> [    0.540943] aspeed-smc 1e620000.spi: CE1 window [ 0x28000000 - 0x2c000000 ] 64MB
> [    0.541143] aspeed-smc 1e620000.spi: read control register: 203b0041
> [    0.581442] 5 fixed-partitions partitions found on MTD device bmc
> [    0.581625] Creating 5 MTD partitions on "bmc":
> [    0.581854] 0x000000000000-0x0000000e0000 : "u-boot"
> [    0.584472] 0x0000000e0000-0x000000100000 : "u-boot-env"
> [    0.586468] 0x000000100000-0x000000a00000 : "kernel"
> [    0.588465] 0x000000a00000-0x000006000000 : "rofs"
> [    0.590552] 0x000006000000-0x000008000000 : "rwfs"
> [    0.592605] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
> [    0.592801] aspeed-smc 1e620000.spi: unrecognized JEDEC id bytes: 00 00 00 00 00 00
> [    0.593039] Deleting MTD partitions on "bmc":
> [    0.593175] Deleting u-boot MTD partition
> [    0.637929] Deleting u-boot-env MTD partition
> [    0.829527] Deleting kernel MTD partition
> [    0.856902] Freeing initrd memory: 1032K
> [    0.866428] Deleting rofs MTD partition
> [    0.906264] Deleting rwfs MTD partition
> [    0.986628] aspeed-smc 1e620000.spi: Aspeed SMC probe failed -2
> [    0.986929] aspeed-smc: probe of 1e620000.spi failed with error -2
> ...
> [    2.936719] /dev/mtdblock: Can't open blockdev
> mount: mounting /dev/mtdblock on run/initramfs/ro failed: No such file or directory
> [    2.963030] MTD: Couldn't look up '/dev/mtdblock': -2
> mount: mounting /dev/mtdblock on run/initramfs/rw failed: No such file or directory
> 
> Mounting read-write /dev/mtdblock filesystem failed.  Please fix and run
> 	mount /dev/mtdblock run/initramfs/rw -t jffs2 -o rw
> or perform a factory reset with the clean-rwfs-filesystem option.
> Fatal error, triggering kernel panic!
> [    3.013047] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000100
> 
> Many BMC designs have two flash chips so that they can handle a hardware
> failure of one of them.  If one chip failed, it doesn't do any good to
> have redundancy if they all get removed anyhow.
> 
> Improve the resilience of the probe function to handle one of the
> children being missing or failed.  Only in the case where all children
> fail to probe should the controller be failed out.
> 
> Signed-off-by: Patrick Williams <patrick@stwcx.xyz>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.

Miquel

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Miquel Raynal]

On Wed, 2021-12-29 at 14:33:33 UTC, Patrick Williams wrote:
> The aspeed-smc can have multiple SPI devices attached to it in the
> device tree.  If one of the devices is missing or failing the entire
> probe will fail and all MTD devices under the controller will be
> removed.  On OpenBMC this results in a kernel panic due to missing
> rootfs:
> 
> [    0.538774] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
> [    0.540471] aspeed-smc 1e620000.spi: w25q01jv-iq (131072 Kbytes)
> [    0.540750] aspeed-smc 1e620000.spi: CE0 window [ 0x20000000 - 0x28000000 ] 128MB
> [    0.540943] aspeed-smc 1e620000.spi: CE1 window [ 0x28000000 - 0x2c000000 ] 64MB
> [    0.541143] aspeed-smc 1e620000.spi: read control register: 203b0041
> [    0.581442] 5 fixed-partitions partitions found on MTD device bmc
> [    0.581625] Creating 5 MTD partitions on "bmc":
> [    0.581854] 0x000000000000-0x0000000e0000 : "u-boot"
> [    0.584472] 0x0000000e0000-0x000000100000 : "u-boot-env"
> [    0.586468] 0x000000100000-0x000000a00000 : "kernel"
> [    0.588465] 0x000000a00000-0x000006000000 : "rofs"
> [    0.590552] 0x000006000000-0x000008000000 : "rwfs"
> [    0.592605] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
> [    0.592801] aspeed-smc 1e620000.spi: unrecognized JEDEC id bytes: 00 00 00 00 00 00
> [    0.593039] Deleting MTD partitions on "bmc":
> [    0.593175] Deleting u-boot MTD partition
> [    0.637929] Deleting u-boot-env MTD partition
> [    0.829527] Deleting kernel MTD partition
> [    0.856902] Freeing initrd memory: 1032K
> [    0.866428] Deleting rofs MTD partition
> [    0.906264] Deleting rwfs MTD partition
> [    0.986628] aspeed-smc 1e620000.spi: Aspeed SMC probe failed -2
> [    0.986929] aspeed-smc: probe of 1e620000.spi failed with error -2
> ...
> [    2.936719] /dev/mtdblock: Can't open blockdev
> mount: mounting /dev/mtdblock on run/initramfs/ro failed: No such file or directory
> [    2.963030] MTD: Couldn't look up '/dev/mtdblock': -2
> mount: mounting /dev/mtdblock on run/initramfs/rw failed: No such file or directory
> 
> Mounting read-write /dev/mtdblock filesystem failed.  Please fix and run
> 	mount /dev/mtdblock run/initramfs/rw -t jffs2 -o rw
> or perform a factory reset with the clean-rwfs-filesystem option.
> Fatal error, triggering kernel panic!
> [    3.013047] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000100
> 
> Many BMC designs have two flash chips so that they can handle a hardware
> failure of one of them.  If one chip failed, it doesn't do any good to
> have redundancy if they all get removed anyhow.
> 
> Improve the resilience of the probe function to handle one of the
> children being missing or failed.  Only in the case where all children
> fail to probe should the controller be failed out.
> 
> Signed-off-by: Patrick Williams <patrick@stwcx.xyz>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.

Miquel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Miquel Raynal]

On Wed, 2021-12-29 at 14:33:33 UTC, Patrick Williams wrote:
> The aspeed-smc can have multiple SPI devices attached to it in the
> device tree.  If one of the devices is missing or failing the entire
> probe will fail and all MTD devices under the controller will be
> removed.  On OpenBMC this results in a kernel panic due to missing
> rootfs:
> 
> [    0.538774] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
> [    0.540471] aspeed-smc 1e620000.spi: w25q01jv-iq (131072 Kbytes)
> [    0.540750] aspeed-smc 1e620000.spi: CE0 window [ 0x20000000 - 0x28000000 ] 128MB
> [    0.540943] aspeed-smc 1e620000.spi: CE1 window [ 0x28000000 - 0x2c000000 ] 64MB
> [    0.541143] aspeed-smc 1e620000.spi: read control register: 203b0041
> [    0.581442] 5 fixed-partitions partitions found on MTD device bmc
> [    0.581625] Creating 5 MTD partitions on "bmc":
> [    0.581854] 0x000000000000-0x0000000e0000 : "u-boot"
> [    0.584472] 0x0000000e0000-0x000000100000 : "u-boot-env"
> [    0.586468] 0x000000100000-0x000000a00000 : "kernel"
> [    0.588465] 0x000000a00000-0x000006000000 : "rofs"
> [    0.590552] 0x000006000000-0x000008000000 : "rwfs"
> [    0.592605] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
> [    0.592801] aspeed-smc 1e620000.spi: unrecognized JEDEC id bytes: 00 00 00 00 00 00
> [    0.593039] Deleting MTD partitions on "bmc":
> [    0.593175] Deleting u-boot MTD partition
> [    0.637929] Deleting u-boot-env MTD partition
> [    0.829527] Deleting kernel MTD partition
> [    0.856902] Freeing initrd memory: 1032K
> [    0.866428] Deleting rofs MTD partition
> [    0.906264] Deleting rwfs MTD partition
> [    0.986628] aspeed-smc 1e620000.spi: Aspeed SMC probe failed -2
> [    0.986929] aspeed-smc: probe of 1e620000.spi failed with error -2
> ...
> [    2.936719] /dev/mtdblock: Can't open blockdev
> mount: mounting /dev/mtdblock on run/initramfs/ro failed: No such file or directory
> [    2.963030] MTD: Couldn't look up '/dev/mtdblock': -2
> mount: mounting /dev/mtdblock on run/initramfs/rw failed: No such file or directory
> 
> Mounting read-write /dev/mtdblock filesystem failed.  Please fix and run
> 	mount /dev/mtdblock run/initramfs/rw -t jffs2 -o rw
> or perform a factory reset with the clean-rwfs-filesystem option.
> Fatal error, triggering kernel panic!
> [    3.013047] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000100
> 
> Many BMC designs have two flash chips so that they can handle a hardware
> failure of one of them.  If one chip failed, it doesn't do any good to
> have redundancy if they all get removed anyhow.
> 
> Improve the resilience of the probe function to handle one of the
> children being missing or failed.  Only in the case where all children
> fail to probe should the controller be failed out.
> 
> Signed-off-by: Patrick Williams <patrick@stwcx.xyz>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.

Miquel

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Cédric Le Goater]

>> I had an offline discussion with someone who knew more history on this driver.
>> My understanding is that the linux-aspeed team is aware of this being deprecated
>> but that there was some missing support for interface training that nobody has
>> gotten around to write?  If that is the case this really isn't even a "simple"
>> port to a new API at this point.
> 
> Unless the controller needs some unique feature (I don't think it does
> on a quick glance), the conversion should not be too difficult. For any
> experienced developer, even if they are unfamiliar with the SPI MEM API,
> I don't think it should take more than 2-3 days to do the conversion.
> The code to program the registers would stay the same, all that needs to
> change is the API through which it is accessed.

Writing a spimem driver is not a problem, I think people have done
that in house. Aspeed has one for AST2600. We have one for u-boot
I wrote sometime ago. I even have one for Linux but training comes
with ugly hacks to fit in the current stack.

All Aspeed SoCs need training and that has been the problem for the
last 4 years or so because we can not do training without knowing
a minimum about the flash being trained :/ The previous framework
offered a way to do a first scan and tune the delay settings
afterwards. It worked pretty well on AST2400, AST2500 and AST2600
even if more complex.

One alternative was to include the setting in the DT but the flash
modules are not always soldered on the boards, at least on OpenPOWER
systems which have sockets for them. The board are large, the wires
long, the need is real, some chips freak out if not tuned correctly.

spimem needs an extension I think. Sorry I have not been able to
push that forward. Lack of time and other tasks to address on the
host side of the machine. This is really a software problem, we
have the HW procedures ready. If a spimem expert could get involved
to make a few proposals, I would be happy to help and do some testing.
QEMU models are good enough for the software part. We can do the
training validation on real HW when ready.

Thanks,

C.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Cédric Le Goater]

>> I had an offline discussion with someone who knew more history on this driver.
>> My understanding is that the linux-aspeed team is aware of this being deprecated
>> but that there was some missing support for interface training that nobody has
>> gotten around to write?  If that is the case this really isn't even a "simple"
>> port to a new API at this point.
> 
> Unless the controller needs some unique feature (I don't think it does
> on a quick glance), the conversion should not be too difficult. For any
> experienced developer, even if they are unfamiliar with the SPI MEM API,
> I don't think it should take more than 2-3 days to do the conversion.
> The code to program the registers would stay the same, all that needs to
> change is the API through which it is accessed.

Writing a spimem driver is not a problem, I think people have done
that in house. Aspeed has one for AST2600. We have one for u-boot
I wrote sometime ago. I even have one for Linux but training comes
with ugly hacks to fit in the current stack.

All Aspeed SoCs need training and that has been the problem for the
last 4 years or so because we can not do training without knowing
a minimum about the flash being trained :/ The previous framework
offered a way to do a first scan and tune the delay settings
afterwards. It worked pretty well on AST2400, AST2500 and AST2600
even if more complex.

One alternative was to include the setting in the DT but the flash
modules are not always soldered on the boards, at least on OpenPOWER
systems which have sockets for them. The board are large, the wires
long, the need is real, some chips freak out if not tuned correctly.

spimem needs an extension I think. Sorry I have not been able to
push that forward. Lack of time and other tasks to address on the
host side of the machine. This is really a software problem, we
have the HW procedures ready. If a spimem expert could get involved
to make a few proposals, I would be happy to help and do some testing.
QEMU models are good enough for the software part. We can do the
training validation on real HW when ready.

Thanks,

C.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Cédric Le Goater]

>> I had an offline discussion with someone who knew more history on this driver.
>> My understanding is that the linux-aspeed team is aware of this being deprecated
>> but that there was some missing support for interface training that nobody has
>> gotten around to write?  If that is the case this really isn't even a "simple"
>> port to a new API at this point.
> 
> Unless the controller needs some unique feature (I don't think it does
> on a quick glance), the conversion should not be too difficult. For any
> experienced developer, even if they are unfamiliar with the SPI MEM API,
> I don't think it should take more than 2-3 days to do the conversion.
> The code to program the registers would stay the same, all that needs to
> change is the API through which it is accessed.

Writing a spimem driver is not a problem, I think people have done
that in house. Aspeed has one for AST2600. We have one for u-boot
I wrote sometime ago. I even have one for Linux but training comes
with ugly hacks to fit in the current stack.

All Aspeed SoCs need training and that has been the problem for the
last 4 years or so because we can not do training without knowing
a minimum about the flash being trained :/ The previous framework
offered a way to do a first scan and tune the delay settings
afterwards. It worked pretty well on AST2400, AST2500 and AST2600
even if more complex.

One alternative was to include the setting in the DT but the flash
modules are not always soldered on the boards, at least on OpenPOWER
systems which have sockets for them. The board are large, the wires
long, the need is real, some chips freak out if not tuned correctly.

spimem needs an extension I think. Sorry I have not been able to
push that forward. Lack of time and other tasks to address on the
host side of the machine. This is really a software problem, we
have the HW procedures ready. If a spimem expert could get involved
to make a few proposals, I would be happy to help and do some testing.
QEMU models are good enough for the software part. We can do the
training validation on real HW when ready.

Thanks,

C.

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Pratyush Yadav]

On 23/01/22 11:44PM, Cédric Le Goater wrote:
> > > I had an offline discussion with someone who knew more history on this driver.
> > > My understanding is that the linux-aspeed team is aware of this being deprecated
> > > but that there was some missing support for interface training that nobody has
> > > gotten around to write?  If that is the case this really isn't even a "simple"
> > > port to a new API at this point.
> > 
> > Unless the controller needs some unique feature (I don't think it does
> > on a quick glance), the conversion should not be too difficult. For any
> > experienced developer, even if they are unfamiliar with the SPI MEM API,
> > I don't think it should take more than 2-3 days to do the conversion.
> > The code to program the registers would stay the same, all that needs to
> > change is the API through which it is accessed.
> 
> Writing a spimem driver is not a problem, I think people have done
> that in house. Aspeed has one for AST2600. We have one for u-boot
> I wrote sometime ago. I even have one for Linux but training comes
> with ugly hacks to fit in the current stack.
> 
> All Aspeed SoCs need training and that has been the problem for the
> last 4 years or so because we can not do training without knowing
> a minimum about the flash being trained :/ The previous framework
> offered a way to do a first scan and tune the delay settings
> afterwards. It worked pretty well on AST2400, AST2500 and AST2600
> even if more complex.
> 
> One alternative was to include the setting in the DT but the flash
> modules are not always soldered on the boards, at least on OpenPOWER
> systems which have sockets for them. The board are large, the wires
> long, the need is real, some chips freak out if not tuned correctly.
> 
> spimem needs an extension I think. Sorry I have not been able to
> push that forward. Lack of time and other tasks to address on the
> host side of the machine. This is really a software problem, we
> have the HW procedures ready. If a spimem expert could get involved
> to make a few proposals, I would be happy to help and do some testing.
> QEMU models are good enough for the software part. We can do the
> training validation on real HW when ready.

What information about the flash do you need for this training? I 
proposed a patch series [0] some time ago trying to implement training 
for TI SoCs. It did not get merged but I do intend to respin it and get 
it through. Would this API work for your tuning as well?

Also, I am curious how your training works. What data do you read for 
training delays? Where is it stored? In our case we need to flash a 
known pattern at some location (which is passed in via DT). Do you need 
to run it for every read transaction or just once after the flash is 
initialized?

[0] https://patchwork.ozlabs.org/project/linux-mtd/list/?series=233504&state=%2A&archive=both

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Pratyush Yadav]

On 23/01/22 11:44PM, Cédric Le Goater wrote:
> > > I had an offline discussion with someone who knew more history on this driver.
> > > My understanding is that the linux-aspeed team is aware of this being deprecated
> > > but that there was some missing support for interface training that nobody has
> > > gotten around to write?  If that is the case this really isn't even a "simple"
> > > port to a new API at this point.
> > 
> > Unless the controller needs some unique feature (I don't think it does
> > on a quick glance), the conversion should not be too difficult. For any
> > experienced developer, even if they are unfamiliar with the SPI MEM API,
> > I don't think it should take more than 2-3 days to do the conversion.
> > The code to program the registers would stay the same, all that needs to
> > change is the API through which it is accessed.
> 
> Writing a spimem driver is not a problem, I think people have done
> that in house. Aspeed has one for AST2600. We have one for u-boot
> I wrote sometime ago. I even have one for Linux but training comes
> with ugly hacks to fit in the current stack.
> 
> All Aspeed SoCs need training and that has been the problem for the
> last 4 years or so because we can not do training without knowing
> a minimum about the flash being trained :/ The previous framework
> offered a way to do a first scan and tune the delay settings
> afterwards. It worked pretty well on AST2400, AST2500 and AST2600
> even if more complex.
> 
> One alternative was to include the setting in the DT but the flash
> modules are not always soldered on the boards, at least on OpenPOWER
> systems which have sockets for them. The board are large, the wires
> long, the need is real, some chips freak out if not tuned correctly.
> 
> spimem needs an extension I think. Sorry I have not been able to
> push that forward. Lack of time and other tasks to address on the
> host side of the machine. This is really a software problem, we
> have the HW procedures ready. If a spimem expert could get involved
> to make a few proposals, I would be happy to help and do some testing.
> QEMU models are good enough for the software part. We can do the
> training validation on real HW when ready.

What information about the flash do you need for this training? I 
proposed a patch series [0] some time ago trying to implement training 
for TI SoCs. It did not get merged but I do intend to respin it and get 
it through. Would this API work for your tuning as well?

Also, I am curious how your training works. What data do you read for 
training delays? Where is it stored? In our case we need to flash a 
known pattern at some location (which is passed in via DT). Do you need 
to run it for every read transaction or just once after the flash is 
initialized?

[0] https://patchwork.ozlabs.org/project/linux-mtd/list/?series=233504&state=%2A&archive=both

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Pratyush Yadav]

On 23/01/22 11:44PM, Cédric Le Goater wrote:
> > > I had an offline discussion with someone who knew more history on this driver.
> > > My understanding is that the linux-aspeed team is aware of this being deprecated
> > > but that there was some missing support for interface training that nobody has
> > > gotten around to write?  If that is the case this really isn't even a "simple"
> > > port to a new API at this point.
> > 
> > Unless the controller needs some unique feature (I don't think it does
> > on a quick glance), the conversion should not be too difficult. For any
> > experienced developer, even if they are unfamiliar with the SPI MEM API,
> > I don't think it should take more than 2-3 days to do the conversion.
> > The code to program the registers would stay the same, all that needs to
> > change is the API through which it is accessed.
> 
> Writing a spimem driver is not a problem, I think people have done
> that in house. Aspeed has one for AST2600. We have one for u-boot
> I wrote sometime ago. I even have one for Linux but training comes
> with ugly hacks to fit in the current stack.
> 
> All Aspeed SoCs need training and that has been the problem for the
> last 4 years or so because we can not do training without knowing
> a minimum about the flash being trained :/ The previous framework
> offered a way to do a first scan and tune the delay settings
> afterwards. It worked pretty well on AST2400, AST2500 and AST2600
> even if more complex.
> 
> One alternative was to include the setting in the DT but the flash
> modules are not always soldered on the boards, at least on OpenPOWER
> systems which have sockets for them. The board are large, the wires
> long, the need is real, some chips freak out if not tuned correctly.
> 
> spimem needs an extension I think. Sorry I have not been able to
> push that forward. Lack of time and other tasks to address on the
> host side of the machine. This is really a software problem, we
> have the HW procedures ready. If a spimem expert could get involved
> to make a few proposals, I would be happy to help and do some testing.
> QEMU models are good enough for the software part. We can do the
> training validation on real HW when ready.

What information about the flash do you need for this training? I 
proposed a patch series [0] some time ago trying to implement training 
for TI SoCs. It did not get merged but I do intend to respin it and get 
it through. Would this API work for your tuning as well?

Also, I am curious how your training works. What data do you read for 
training delays? Where is it stored? In our case we need to flash a 
known pattern at some location (which is passed in via DT). Do you need 
to run it for every read transaction or just once after the flash is 
initialized?

[0] https://patchwork.ozlabs.org/project/linux-mtd/list/?series=233504&state=%2A&archive=both

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Cédric Le Goater]

>> spimem needs an extension I think. Sorry I have not been able to
>> push that forward. Lack of time and other tasks to address on the
>> host side of the machine. This is really a software problem, we
>> have the HW procedures ready. If a spimem expert could get involved
>> to make a few proposals, I would be happy to help and do some testing.
>> QEMU models are good enough for the software part. We can do the
>> training validation on real HW when ready.
> 
> What information about the flash do you need for this training? 

Last time I looked, we lacked some post_init handler to setup a slave:
configure the registers defining the AHB windows for each flash
slave and perform the read timing calibration. calibration should
only be done once.

See how the aspeed_spi_flash_init() routine doing the calibration
is hooked up under aspeed_spi_claim_bus() in the u-boot driver :

   https://github.com/openbmc/u-boot/blob/v2019.04-aspeed-openbmc/drivers/spi/aspeed_spi.c

Not good enough for upstream, Linux would be the same :/

> I proposed a patch series [0] some time ago trying to implement training
> for TI SoCs. It did not get merged but I do intend to respin it and get
> it through. Would this API work for your tuning as well?

I will take a look.
  
> Also, I am curious how your training works. What data do you read for
> training delays? Where is it stored? 

The driver reads the first 16K at slow speed (that's why we need a
basic minimal setup of the slave) and checks if the buffer is valid
enough for the calibration :

   https://github.com/openbmc/linux/blob/dev-5.15/drivers/mtd/spi-nor/controllers/aspeed-smc.c#L998

it then performs reads by changing the frequency and delays and
compares results with the initial default buffer.

if not, then the driver stays in a safe mode (slow).

> In our case we need to flash a
> known pattern at some location (which is passed in via DT). Do you need
> to run it for every read transaction or just once after the flash is
> initialized?

Just once because it is a heavy process. See the debug outputs below.
Once we have good read timings and frequency, there is no need to do
it each time.

> [0] https://patchwork.ozlabs.org/project/linux-mtd/list/?series=233504&state=%2A&archive=both
Thanks,

C.



There are 3 controllers, 1e620000/FMC is for the BMC. We keep
safe settings for it and normally u-boot has done the training
already . The other two controllers are for the SPI-NOR of the
host and for these we push the frequency higher.


AST2600 EVB:

[    0.689662] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
[    0.696412] aspeed-smc 1e620000.spi: control register: 203b0641
[    0.696426] aspeed-smc 1e620000.spi: control register changed to: 00000600
[    0.696434] aspeed-smc 1e620000.spi: default control register: 00000600
[    0.696616] aspeed-smc 1e620000.spi: w25q512jv (65536 Kbytes)
[    0.703108] aspeed-smc 1e620000.spi: CE0 window [ 0x20000000 - 0x24000000 ] 64MB
[    0.711445] aspeed-smc 1e620000.spi: CE1 window [ 0x24000000 - 0x2c000000 ] 128MB
[    0.719864] aspeed-smc 1e620000.spi: write control register: 00120602
[    0.719873] aspeed-smc 1e620000.spi: read control register: 203c0641
[    0.727026] aspeed-smc 1e620000.spi: AHB frequency: 187 MHz
[    0.739247] aspeed-smc 1e620000.spi: Trying HCLK/5 [203c0d41] ...
[    0.767181] aspeed-smc 1e620000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    0.767196] aspeed-smc 1e620000.spi: Trying HCLK/4 [203c0641] ...
[    0.791559] aspeed-smc 1e620000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    0.791571] aspeed-smc 1e620000.spi: Found good read timings at HCLK/4
[    0.795729] 5 fixed-partitions partitions found on MTD device bmc
[    0.802636] Creating 5 MTD partitions on "bmc":
[    0.807739] 0x000000000000-0x0000000e0000 : "u-boot"
[    0.814367] 0x0000000e0000-0x000000100000 : "u-boot-env"
[    0.821306] 0x000000100000-0x000000a00000 : "kernel"
[    0.827755] 0x000000a00000-0x000002a00000 : "rofs"
[    0.834051] 0x000002a00000-0x000004000000 : "rwfs"
[    0.844040] aspeed-smc 1e630000.spi: Using 100 MHz SPI frequency
[    0.850912] aspeed-smc 1e630000.spi: control register: 00000400
[    0.850927] aspeed-smc 1e630000.spi: default control register: 00000400
[    0.851152] aspeed-smc 1e630000.spi: w25q256 (32768 Kbytes)
[    0.857427] aspeed-smc 1e630000.spi: CE0 window [ 0x30000000 - 0x32000000 ] 32MB
[    0.865792] aspeed-smc 1e630000.spi: CE1 window [ 0x32000000 - 0x32000000 ] 0MB (disabled)
[    0.875129] aspeed-smc 1e630000.spi: write control register: 00120402
[    0.875142] aspeed-smc 1e630000.spi: read control register: 203c0441
[    0.882296] aspeed-smc 1e630000.spi: AHB frequency: 187 MHz
[    0.894509] aspeed-smc 1e630000.spi: Trying HCLK/5 [203c0d41] ...
[    0.922417] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    0.922432] aspeed-smc 1e630000.spi: Trying HCLK/4 [203c0641] ...
[    0.946791] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    0.946803] aspeed-smc 1e630000.spi: Trying HCLK/3 [203c0e41] ...
[    0.967644] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    0.967655] aspeed-smc 1e630000.spi: Trying HCLK/2 [203c0741] ...
[    0.969325] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, DI delay none : FAIL
[    0.971007] aspeed-smc 1e630000.spi:   * [00000008] 0 HCLK delay, DI delay 0.5ns : FAIL
[    0.972679] aspeed-smc 1e630000.spi:   * [00000018] 0 HCLK delay, DI delay 1.5ns : FAIL
[    0.974350] aspeed-smc 1e630000.spi:   * [00000028] 0 HCLK delay, DI delay 1.5ns : FAIL
[    0.976021] aspeed-smc 1e630000.spi:   * [00000038] 0 HCLK delay, DI delay 2.5ns : FAIL
[    0.977692] aspeed-smc 1e630000.spi:   * [00000048] 0 HCLK delay, DI delay 2.5ns : FAIL
[    0.979363] aspeed-smc 1e630000.spi:   * [00000058] 0 HCLK delay, DI delay 3.5ns : FAIL
[    0.981042] aspeed-smc 1e630000.spi:   * [00000068] 0 HCLK delay, DI delay 3.5ns : FAIL
[    0.982714] aspeed-smc 1e630000.spi:   * [00000078] 0 HCLK delay, DI delay 4.5ns : FAIL
[    0.984385] aspeed-smc 1e630000.spi:   * [00000088] 0 HCLK delay, DI delay 4.5ns : FAIL
[    0.986056] aspeed-smc 1e630000.spi:   * [00000098] 0 HCLK delay, DI delay 5.5ns : FAIL
[    0.987727] aspeed-smc 1e630000.spi:   * [000000a8] 0 HCLK delay, DI delay 5.5ns : FAIL
[    0.989397] aspeed-smc 1e630000.spi:   * [000000b8] 0 HCLK delay, DI delay 6.5ns : FAIL
[    0.991084] aspeed-smc 1e630000.spi:   * [000000c8] 0 HCLK delay, DI delay 6.5ns : FAIL
[    0.992757] aspeed-smc 1e630000.spi:   * [000000d8] 0 HCLK delay, DI delay 7.5ns : FAIL
[    0.994428] aspeed-smc 1e630000.spi:   * [000000e8] 0 HCLK delay, DI delay 7.5ns : FAIL
[    0.996099] aspeed-smc 1e630000.spi:   * [000000f8] 0 HCLK delay, DI delay 8.5ns : FAIL
[    1.013874] aspeed-smc 1e630000.spi:   * [000000f1] 1 HCLK delay, DI delay none : PASS
[    1.013885] aspeed-smc 1e630000.spi: Found good read timings at HCLK/2
[    1.021498] aspeed-smc 1e631000.spi: Using 100 MHz SPI frequency
[    1.028291] aspeed-smc 1e631000.spi: control register: 00000400
[    1.028302] aspeed-smc 1e631000.spi: default control register: 00000400
[    1.028510] aspeed-smc 1e631000.spi: w25q256 (32768 Kbytes)
[    1.034848] aspeed-smc 1e631000.spi: CE0 window [ 0x50000000 - 0x52000000 ] 32MB
[    1.043197] aspeed-smc 1e631000.spi: CE1 window [ 0x52000000 - 0x52000000 ] 0MB (disabled)
[    1.052518] aspeed-smc 1e631000.spi: write control register: 00120402
[    1.052530] aspeed-smc 1e631000.spi: read control register: 203c0441
[    1.059677] aspeed-smc 1e631000.spi: AHB frequency: 187 MHz
[    1.071900] aspeed-smc 1e631000.spi: Trying HCLK/5 [203c0d41] ...
[    1.099805] aspeed-smc 1e631000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    1.099817] aspeed-smc 1e631000.spi: Trying HCLK/4 [203c0641] ...
[    1.124202] aspeed-smc 1e631000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    1.124219] aspeed-smc 1e631000.spi: Trying HCLK/3 [203c0e41] ...
[    1.145070] aspeed-smc 1e631000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    1.145082] aspeed-smc 1e631000.spi: Trying HCLK/2 [203c0741] ...
[    1.146752] aspeed-smc 1e631000.spi:   * [00000000] 0 HCLK delay, DI delay none : FAIL
[    1.148422] aspeed-smc 1e631000.spi:   * [00000008] 0 HCLK delay, DI delay 0.5ns : FAIL
[    1.150093] aspeed-smc 1e631000.spi:   * [00000018] 0 HCLK delay, DI delay 1.5ns : FAIL
[    1.151778] aspeed-smc 1e631000.spi:   * [00000028] 0 HCLK delay, DI delay 1.5ns : FAIL
[    1.153451] aspeed-smc 1e631000.spi:   * [00000038] 0 HCLK delay, DI delay 2.5ns : FAIL
[    1.155122] aspeed-smc 1e631000.spi:   * [00000048] 0 HCLK delay, DI delay 2.5ns : FAIL
[    1.156793] aspeed-smc 1e631000.spi:   * [00000058] 0 HCLK delay, DI delay 3.5ns : FAIL
[    1.158464] aspeed-smc 1e631000.spi:   * [00000068] 0 HCLK delay, DI delay 3.5ns : FAIL
[    1.160135] aspeed-smc 1e631000.spi:   * [00000078] 0 HCLK delay, DI delay 4.5ns : FAIL
[    1.161818] aspeed-smc 1e631000.spi:   * [00000088] 0 HCLK delay, DI delay 4.5ns : FAIL
[    1.163490] aspeed-smc 1e631000.spi:   * [00000098] 0 HCLK delay, DI delay 5.5ns : FAIL
[    1.165161] aspeed-smc 1e631000.spi:   * [000000a8] 0 HCLK delay, DI delay 5.5ns : FAIL
[    1.166833] aspeed-smc 1e631000.spi:   * [000000b8] 0 HCLK delay, DI delay 6.5ns : FAIL
[    1.168504] aspeed-smc 1e631000.spi:   * [000000c8] 0 HCLK delay, DI delay 6.5ns : FAIL
[    1.170175] aspeed-smc 1e631000.spi:   * [000000d8] 0 HCLK delay, DI delay 7.5ns : FAIL
[    1.171863] aspeed-smc 1e631000.spi:   * [000000e8] 0 HCLK delay, DI delay 7.5ns : FAIL
[    1.173536] aspeed-smc 1e631000.spi:   * [000000f8] 0 HCLK delay, DI delay 8.5ns : FAIL
[    1.191318] aspeed-smc 1e631000.spi:   * [000000f1] 1 HCLK delay, DI delay none : PASS
[    1.191330] aspeed-smc 1e631000.spi: Found good read timings at HCLK/2


an ASTS2500 EVB :

[    1.220804] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
[    1.226797] aspeed-smc 1e620000.spi: control register: 000b0641
[    1.226836] aspeed-smc 1e620000.spi: control register changed to: 00000600
[    1.226860] aspeed-smc 1e620000.spi: default control register: 00000600
[    1.227092] aspeed-smc 1e620000.spi: w25q256 (32768 Kbytes)
[    1.232806] aspeed-smc 1e620000.spi: CE0 window [ 0x20000000 - 0x22000000 ] 32MB
[    1.240329] aspeed-smc 1e620000.spi: CE1 window [ 0x22000000 - 0x2a000000 ] 128MB
[    1.247852] aspeed-smc 1e620000.spi: write control register: 00020602
[    1.247882] aspeed-smc 1e620000.spi: read control register: 203b0641
[    1.254315] aspeed-smc 1e620000.spi: AHB frequency: 198 MHz
[    1.265406] aspeed-smc 1e620000.spi: Trying HCLK/5 [203b0d41] ...
[    1.287111] aspeed-smc 1e620000.spi:   * [00080000] 0 HCLK delay, 4ns DI delay : PASS
[    1.309048] aspeed-smc 1e620000.spi:   * [00000000] 0 HCLK delay, 0ns DI delay : PASS
[    1.331223] aspeed-smc 1e620000.spi:   * [00090000] 1 HCLK delay, 4ns DI delay : PASS
[    1.331278] aspeed-smc 1e620000.spi:  * -> good is pass 1 [0x00000000]
[    1.331308] aspeed-smc 1e620000.spi: Trying HCLK/4 [203b0641] ...
[    1.349958] aspeed-smc 1e620000.spi:   * [00008000] 0 HCLK delay, 4ns DI delay : PASS
[    1.368473] aspeed-smc 1e620000.spi:   * [00000000] 0 HCLK delay, 0ns DI delay : PASS
[    1.387341] aspeed-smc 1e620000.spi:   * [00009000] 1 HCLK delay, 4ns DI delay : PASS
[    1.387397] aspeed-smc 1e620000.spi:  * -> good is pass 1 [0x00000000]
[    1.387435] aspeed-smc 1e620000.spi: Found good read timings at HCLK/4
[    1.858947] Freeing initrd memory: 1044K
[    1.906913] 5 fixed-partitions partitions found on MTD device bmc
[    1.913143] Creating 5 MTD partitions on "bmc":
[    1.917724] 0x000000000000-0x000000060000 : "u-boot"
[    1.925920] 0x000000060000-0x000000080000 : "u-boot-env"
[    1.937262] 0x000000080000-0x0000004c0000 : "kernel"
[    1.948189] 0x0000004c0000-0x000001c00000 : "rofs"
[    1.959196] 0x000001c00000-0x000002000000 : "rwfs"
[    1.971557] aspeed-smc 1e630000.spi: Using 100 MHz SPI frequency
[    1.977632] aspeed-smc 1e630000.spi: control register: 00000200
[    1.977669] aspeed-smc 1e630000.spi: default control register: 00000200
[    1.977961] aspeed-smc 1e630000.spi: w25q256 (32768 Kbytes)
[    1.983674] aspeed-smc 1e630000.spi: CE0 window [ 0x30000000 - 0x32000000 ] 32MB
[    1.991183] aspeed-smc 1e630000.spi: CE1 window [ 0x32000000 - 0x38000000 ] 96MB
[    1.998621] aspeed-smc 1e630000.spi: write control register: 00020202
[    1.998652] aspeed-smc 1e630000.spi: read control register: 203b0241
[    2.005086] aspeed-smc 1e630000.spi: AHB frequency: 198 MHz
[    2.016174] aspeed-smc 1e630000.spi: Trying HCLK/5 [203b0d41] ...
[    2.038011] aspeed-smc 1e630000.spi:   * [00080000] 0 HCLK delay, 4ns DI delay : PASS
[    2.060035] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, 0ns DI delay : PASS
[    2.082211] aspeed-smc 1e630000.spi:   * [00090000] 1 HCLK delay, 4ns DI delay : PASS
[    2.082266] aspeed-smc 1e630000.spi:  * -> good is pass 1 [0x00000000]
[    2.082295] aspeed-smc 1e630000.spi: Trying HCLK/4 [203b0641] ...
[    2.100938] aspeed-smc 1e630000.spi:   * [00008000] 0 HCLK delay, 4ns DI delay : PASS
[    2.119623] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, 0ns DI delay : PASS
[    2.138440] aspeed-smc 1e630000.spi:   * [00009000] 1 HCLK delay, 4ns DI delay : PASS
[    2.138491] aspeed-smc 1e630000.spi:  * -> good is pass 1 [0x00000000]
[    2.138521] aspeed-smc 1e630000.spi: Trying HCLK/3 [203b0e41] ...
[    2.139827] aspeed-smc 1e630000.spi:   * [00000800] 0 HCLK delay, 4ns DI delay : FAIL
[    2.155093] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, 0ns DI delay : PASS
[    2.170627] aspeed-smc 1e630000.spi:   * [00000900] 1 HCLK delay, 4ns DI delay : PASS
[    2.186111] aspeed-smc 1e630000.spi:   * [00000100] 1 HCLK delay, 0ns DI delay : PASS
[    2.186164] aspeed-smc 1e630000.spi:  * -> good is pass 2 [0x00000900]
[    2.186195] aspeed-smc 1e630000.spi: Trying HCLK/2 [203b0741] ...
[    2.187103] aspeed-smc 1e630000.spi:   * [00000080] 0 HCLK delay, 4ns DI delay : FAIL
[    2.188010] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, 0ns DI delay : FAIL
[    2.200197] aspeed-smc 1e630000.spi:   * [00000090] 1 HCLK delay, 4ns DI delay : PASS
[    2.212359] aspeed-smc 1e630000.spi:   * [00000010] 1 HCLK delay, 0ns DI delay : PASS
[    2.224725] aspeed-smc 1e630000.spi:   * [000000a0] 2 HCLK delay, 4ns DI delay : PASS
[    2.224777] aspeed-smc 1e630000.spi:  * -> good is pass 3 [0x00000010]
[    2.224810] aspeed-smc 1e630000.spi: Found good read timings at HCLK/2
[    2.244098] aspeed-smc 1e631000.spi: Aspeed SMC probe failed -19

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Cédric Le Goater]

>> spimem needs an extension I think. Sorry I have not been able to
>> push that forward. Lack of time and other tasks to address on the
>> host side of the machine. This is really a software problem, we
>> have the HW procedures ready. If a spimem expert could get involved
>> to make a few proposals, I would be happy to help and do some testing.
>> QEMU models are good enough for the software part. We can do the
>> training validation on real HW when ready.
> 
> What information about the flash do you need for this training? 

Last time I looked, we lacked some post_init handler to setup a slave:
configure the registers defining the AHB windows for each flash
slave and perform the read timing calibration. calibration should
only be done once.

See how the aspeed_spi_flash_init() routine doing the calibration
is hooked up under aspeed_spi_claim_bus() in the u-boot driver :

   https://github.com/openbmc/u-boot/blob/v2019.04-aspeed-openbmc/drivers/spi/aspeed_spi.c

Not good enough for upstream, Linux would be the same :/

> I proposed a patch series [0] some time ago trying to implement training
> for TI SoCs. It did not get merged but I do intend to respin it and get
> it through. Would this API work for your tuning as well?

I will take a look.
  
> Also, I am curious how your training works. What data do you read for
> training delays? Where is it stored? 

The driver reads the first 16K at slow speed (that's why we need a
basic minimal setup of the slave) and checks if the buffer is valid
enough for the calibration :

   https://github.com/openbmc/linux/blob/dev-5.15/drivers/mtd/spi-nor/controllers/aspeed-smc.c#L998

it then performs reads by changing the frequency and delays and
compares results with the initial default buffer.

if not, then the driver stays in a safe mode (slow).

> In our case we need to flash a
> known pattern at some location (which is passed in via DT). Do you need
> to run it for every read transaction or just once after the flash is
> initialized?

Just once because it is a heavy process. See the debug outputs below.
Once we have good read timings and frequency, there is no need to do
it each time.

> [0] https://patchwork.ozlabs.org/project/linux-mtd/list/?series=233504&state=%2A&archive=both
Thanks,

C.



There are 3 controllers, 1e620000/FMC is for the BMC. We keep
safe settings for it and normally u-boot has done the training
already . The other two controllers are for the SPI-NOR of the
host and for these we push the frequency higher.


AST2600 EVB:

[    0.689662] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
[    0.696412] aspeed-smc 1e620000.spi: control register: 203b0641
[    0.696426] aspeed-smc 1e620000.spi: control register changed to: 00000600
[    0.696434] aspeed-smc 1e620000.spi: default control register: 00000600
[    0.696616] aspeed-smc 1e620000.spi: w25q512jv (65536 Kbytes)
[    0.703108] aspeed-smc 1e620000.spi: CE0 window [ 0x20000000 - 0x24000000 ] 64MB
[    0.711445] aspeed-smc 1e620000.spi: CE1 window [ 0x24000000 - 0x2c000000 ] 128MB
[    0.719864] aspeed-smc 1e620000.spi: write control register: 00120602
[    0.719873] aspeed-smc 1e620000.spi: read control register: 203c0641
[    0.727026] aspeed-smc 1e620000.spi: AHB frequency: 187 MHz
[    0.739247] aspeed-smc 1e620000.spi: Trying HCLK/5 [203c0d41] ...
[    0.767181] aspeed-smc 1e620000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    0.767196] aspeed-smc 1e620000.spi: Trying HCLK/4 [203c0641] ...
[    0.791559] aspeed-smc 1e620000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    0.791571] aspeed-smc 1e620000.spi: Found good read timings at HCLK/4
[    0.795729] 5 fixed-partitions partitions found on MTD device bmc
[    0.802636] Creating 5 MTD partitions on "bmc":
[    0.807739] 0x000000000000-0x0000000e0000 : "u-boot"
[    0.814367] 0x0000000e0000-0x000000100000 : "u-boot-env"
[    0.821306] 0x000000100000-0x000000a00000 : "kernel"
[    0.827755] 0x000000a00000-0x000002a00000 : "rofs"
[    0.834051] 0x000002a00000-0x000004000000 : "rwfs"
[    0.844040] aspeed-smc 1e630000.spi: Using 100 MHz SPI frequency
[    0.850912] aspeed-smc 1e630000.spi: control register: 00000400
[    0.850927] aspeed-smc 1e630000.spi: default control register: 00000400
[    0.851152] aspeed-smc 1e630000.spi: w25q256 (32768 Kbytes)
[    0.857427] aspeed-smc 1e630000.spi: CE0 window [ 0x30000000 - 0x32000000 ] 32MB
[    0.865792] aspeed-smc 1e630000.spi: CE1 window [ 0x32000000 - 0x32000000 ] 0MB (disabled)
[    0.875129] aspeed-smc 1e630000.spi: write control register: 00120402
[    0.875142] aspeed-smc 1e630000.spi: read control register: 203c0441
[    0.882296] aspeed-smc 1e630000.spi: AHB frequency: 187 MHz
[    0.894509] aspeed-smc 1e630000.spi: Trying HCLK/5 [203c0d41] ...
[    0.922417] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    0.922432] aspeed-smc 1e630000.spi: Trying HCLK/4 [203c0641] ...
[    0.946791] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    0.946803] aspeed-smc 1e630000.spi: Trying HCLK/3 [203c0e41] ...
[    0.967644] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    0.967655] aspeed-smc 1e630000.spi: Trying HCLK/2 [203c0741] ...
[    0.969325] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, DI delay none : FAIL
[    0.971007] aspeed-smc 1e630000.spi:   * [00000008] 0 HCLK delay, DI delay 0.5ns : FAIL
[    0.972679] aspeed-smc 1e630000.spi:   * [00000018] 0 HCLK delay, DI delay 1.5ns : FAIL
[    0.974350] aspeed-smc 1e630000.spi:   * [00000028] 0 HCLK delay, DI delay 1.5ns : FAIL
[    0.976021] aspeed-smc 1e630000.spi:   * [00000038] 0 HCLK delay, DI delay 2.5ns : FAIL
[    0.977692] aspeed-smc 1e630000.spi:   * [00000048] 0 HCLK delay, DI delay 2.5ns : FAIL
[    0.979363] aspeed-smc 1e630000.spi:   * [00000058] 0 HCLK delay, DI delay 3.5ns : FAIL
[    0.981042] aspeed-smc 1e630000.spi:   * [00000068] 0 HCLK delay, DI delay 3.5ns : FAIL
[    0.982714] aspeed-smc 1e630000.spi:   * [00000078] 0 HCLK delay, DI delay 4.5ns : FAIL
[    0.984385] aspeed-smc 1e630000.spi:   * [00000088] 0 HCLK delay, DI delay 4.5ns : FAIL
[    0.986056] aspeed-smc 1e630000.spi:   * [00000098] 0 HCLK delay, DI delay 5.5ns : FAIL
[    0.987727] aspeed-smc 1e630000.spi:   * [000000a8] 0 HCLK delay, DI delay 5.5ns : FAIL
[    0.989397] aspeed-smc 1e630000.spi:   * [000000b8] 0 HCLK delay, DI delay 6.5ns : FAIL
[    0.991084] aspeed-smc 1e630000.spi:   * [000000c8] 0 HCLK delay, DI delay 6.5ns : FAIL
[    0.992757] aspeed-smc 1e630000.spi:   * [000000d8] 0 HCLK delay, DI delay 7.5ns : FAIL
[    0.994428] aspeed-smc 1e630000.spi:   * [000000e8] 0 HCLK delay, DI delay 7.5ns : FAIL
[    0.996099] aspeed-smc 1e630000.spi:   * [000000f8] 0 HCLK delay, DI delay 8.5ns : FAIL
[    1.013874] aspeed-smc 1e630000.spi:   * [000000f1] 1 HCLK delay, DI delay none : PASS
[    1.013885] aspeed-smc 1e630000.spi: Found good read timings at HCLK/2
[    1.021498] aspeed-smc 1e631000.spi: Using 100 MHz SPI frequency
[    1.028291] aspeed-smc 1e631000.spi: control register: 00000400
[    1.028302] aspeed-smc 1e631000.spi: default control register: 00000400
[    1.028510] aspeed-smc 1e631000.spi: w25q256 (32768 Kbytes)
[    1.034848] aspeed-smc 1e631000.spi: CE0 window [ 0x50000000 - 0x52000000 ] 32MB
[    1.043197] aspeed-smc 1e631000.spi: CE1 window [ 0x52000000 - 0x52000000 ] 0MB (disabled)
[    1.052518] aspeed-smc 1e631000.spi: write control register: 00120402
[    1.052530] aspeed-smc 1e631000.spi: read control register: 203c0441
[    1.059677] aspeed-smc 1e631000.spi: AHB frequency: 187 MHz
[    1.071900] aspeed-smc 1e631000.spi: Trying HCLK/5 [203c0d41] ...
[    1.099805] aspeed-smc 1e631000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    1.099817] aspeed-smc 1e631000.spi: Trying HCLK/4 [203c0641] ...
[    1.124202] aspeed-smc 1e631000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    1.124219] aspeed-smc 1e631000.spi: Trying HCLK/3 [203c0e41] ...
[    1.145070] aspeed-smc 1e631000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    1.145082] aspeed-smc 1e631000.spi: Trying HCLK/2 [203c0741] ...
[    1.146752] aspeed-smc 1e631000.spi:   * [00000000] 0 HCLK delay, DI delay none : FAIL
[    1.148422] aspeed-smc 1e631000.spi:   * [00000008] 0 HCLK delay, DI delay 0.5ns : FAIL
[    1.150093] aspeed-smc 1e631000.spi:   * [00000018] 0 HCLK delay, DI delay 1.5ns : FAIL
[    1.151778] aspeed-smc 1e631000.spi:   * [00000028] 0 HCLK delay, DI delay 1.5ns : FAIL
[    1.153451] aspeed-smc 1e631000.spi:   * [00000038] 0 HCLK delay, DI delay 2.5ns : FAIL
[    1.155122] aspeed-smc 1e631000.spi:   * [00000048] 0 HCLK delay, DI delay 2.5ns : FAIL
[    1.156793] aspeed-smc 1e631000.spi:   * [00000058] 0 HCLK delay, DI delay 3.5ns : FAIL
[    1.158464] aspeed-smc 1e631000.spi:   * [00000068] 0 HCLK delay, DI delay 3.5ns : FAIL
[    1.160135] aspeed-smc 1e631000.spi:   * [00000078] 0 HCLK delay, DI delay 4.5ns : FAIL
[    1.161818] aspeed-smc 1e631000.spi:   * [00000088] 0 HCLK delay, DI delay 4.5ns : FAIL
[    1.163490] aspeed-smc 1e631000.spi:   * [00000098] 0 HCLK delay, DI delay 5.5ns : FAIL
[    1.165161] aspeed-smc 1e631000.spi:   * [000000a8] 0 HCLK delay, DI delay 5.5ns : FAIL
[    1.166833] aspeed-smc 1e631000.spi:   * [000000b8] 0 HCLK delay, DI delay 6.5ns : FAIL
[    1.168504] aspeed-smc 1e631000.spi:   * [000000c8] 0 HCLK delay, DI delay 6.5ns : FAIL
[    1.170175] aspeed-smc 1e631000.spi:   * [000000d8] 0 HCLK delay, DI delay 7.5ns : FAIL
[    1.171863] aspeed-smc 1e631000.spi:   * [000000e8] 0 HCLK delay, DI delay 7.5ns : FAIL
[    1.173536] aspeed-smc 1e631000.spi:   * [000000f8] 0 HCLK delay, DI delay 8.5ns : FAIL
[    1.191318] aspeed-smc 1e631000.spi:   * [000000f1] 1 HCLK delay, DI delay none : PASS
[    1.191330] aspeed-smc 1e631000.spi: Found good read timings at HCLK/2


an ASTS2500 EVB :

[    1.220804] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
[    1.226797] aspeed-smc 1e620000.spi: control register: 000b0641
[    1.226836] aspeed-smc 1e620000.spi: control register changed to: 00000600
[    1.226860] aspeed-smc 1e620000.spi: default control register: 00000600
[    1.227092] aspeed-smc 1e620000.spi: w25q256 (32768 Kbytes)
[    1.232806] aspeed-smc 1e620000.spi: CE0 window [ 0x20000000 - 0x22000000 ] 32MB
[    1.240329] aspeed-smc 1e620000.spi: CE1 window [ 0x22000000 - 0x2a000000 ] 128MB
[    1.247852] aspeed-smc 1e620000.spi: write control register: 00020602
[    1.247882] aspeed-smc 1e620000.spi: read control register: 203b0641
[    1.254315] aspeed-smc 1e620000.spi: AHB frequency: 198 MHz
[    1.265406] aspeed-smc 1e620000.spi: Trying HCLK/5 [203b0d41] ...
[    1.287111] aspeed-smc 1e620000.spi:   * [00080000] 0 HCLK delay, 4ns DI delay : PASS
[    1.309048] aspeed-smc 1e620000.spi:   * [00000000] 0 HCLK delay, 0ns DI delay : PASS
[    1.331223] aspeed-smc 1e620000.spi:   * [00090000] 1 HCLK delay, 4ns DI delay : PASS
[    1.331278] aspeed-smc 1e620000.spi:  * -> good is pass 1 [0x00000000]
[    1.331308] aspeed-smc 1e620000.spi: Trying HCLK/4 [203b0641] ...
[    1.349958] aspeed-smc 1e620000.spi:   * [00008000] 0 HCLK delay, 4ns DI delay : PASS
[    1.368473] aspeed-smc 1e620000.spi:   * [00000000] 0 HCLK delay, 0ns DI delay : PASS
[    1.387341] aspeed-smc 1e620000.spi:   * [00009000] 1 HCLK delay, 4ns DI delay : PASS
[    1.387397] aspeed-smc 1e620000.spi:  * -> good is pass 1 [0x00000000]
[    1.387435] aspeed-smc 1e620000.spi: Found good read timings at HCLK/4
[    1.858947] Freeing initrd memory: 1044K
[    1.906913] 5 fixed-partitions partitions found on MTD device bmc
[    1.913143] Creating 5 MTD partitions on "bmc":
[    1.917724] 0x000000000000-0x000000060000 : "u-boot"
[    1.925920] 0x000000060000-0x000000080000 : "u-boot-env"
[    1.937262] 0x000000080000-0x0000004c0000 : "kernel"
[    1.948189] 0x0000004c0000-0x000001c00000 : "rofs"
[    1.959196] 0x000001c00000-0x000002000000 : "rwfs"
[    1.971557] aspeed-smc 1e630000.spi: Using 100 MHz SPI frequency
[    1.977632] aspeed-smc 1e630000.spi: control register: 00000200
[    1.977669] aspeed-smc 1e630000.spi: default control register: 00000200
[    1.977961] aspeed-smc 1e630000.spi: w25q256 (32768 Kbytes)
[    1.983674] aspeed-smc 1e630000.spi: CE0 window [ 0x30000000 - 0x32000000 ] 32MB
[    1.991183] aspeed-smc 1e630000.spi: CE1 window [ 0x32000000 - 0x38000000 ] 96MB
[    1.998621] aspeed-smc 1e630000.spi: write control register: 00020202
[    1.998652] aspeed-smc 1e630000.spi: read control register: 203b0241
[    2.005086] aspeed-smc 1e630000.spi: AHB frequency: 198 MHz
[    2.016174] aspeed-smc 1e630000.spi: Trying HCLK/5 [203b0d41] ...
[    2.038011] aspeed-smc 1e630000.spi:   * [00080000] 0 HCLK delay, 4ns DI delay : PASS
[    2.060035] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, 0ns DI delay : PASS
[    2.082211] aspeed-smc 1e630000.spi:   * [00090000] 1 HCLK delay, 4ns DI delay : PASS
[    2.082266] aspeed-smc 1e630000.spi:  * -> good is pass 1 [0x00000000]
[    2.082295] aspeed-smc 1e630000.spi: Trying HCLK/4 [203b0641] ...
[    2.100938] aspeed-smc 1e630000.spi:   * [00008000] 0 HCLK delay, 4ns DI delay : PASS
[    2.119623] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, 0ns DI delay : PASS
[    2.138440] aspeed-smc 1e630000.spi:   * [00009000] 1 HCLK delay, 4ns DI delay : PASS
[    2.138491] aspeed-smc 1e630000.spi:  * -> good is pass 1 [0x00000000]
[    2.138521] aspeed-smc 1e630000.spi: Trying HCLK/3 [203b0e41] ...
[    2.139827] aspeed-smc 1e630000.spi:   * [00000800] 0 HCLK delay, 4ns DI delay : FAIL
[    2.155093] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, 0ns DI delay : PASS
[    2.170627] aspeed-smc 1e630000.spi:   * [00000900] 1 HCLK delay, 4ns DI delay : PASS
[    2.186111] aspeed-smc 1e630000.spi:   * [00000100] 1 HCLK delay, 0ns DI delay : PASS
[    2.186164] aspeed-smc 1e630000.spi:  * -> good is pass 2 [0x00000900]
[    2.186195] aspeed-smc 1e630000.spi: Trying HCLK/2 [203b0741] ...
[    2.187103] aspeed-smc 1e630000.spi:   * [00000080] 0 HCLK delay, 4ns DI delay : FAIL
[    2.188010] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, 0ns DI delay : FAIL
[    2.200197] aspeed-smc 1e630000.spi:   * [00000090] 1 HCLK delay, 4ns DI delay : PASS
[    2.212359] aspeed-smc 1e630000.spi:   * [00000010] 1 HCLK delay, 0ns DI delay : PASS
[    2.224725] aspeed-smc 1e630000.spi:   * [000000a0] 2 HCLK delay, 4ns DI delay : PASS
[    2.224777] aspeed-smc 1e630000.spi:  * -> good is pass 3 [0x00000010]
[    2.224810] aspeed-smc 1e630000.spi: Found good read timings at HCLK/2
[    2.244098] aspeed-smc 1e631000.spi: Aspeed SMC probe failed -19

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Cédric Le Goater]

>> spimem needs an extension I think. Sorry I have not been able to
>> push that forward. Lack of time and other tasks to address on the
>> host side of the machine. This is really a software problem, we
>> have the HW procedures ready. If a spimem expert could get involved
>> to make a few proposals, I would be happy to help and do some testing.
>> QEMU models are good enough for the software part. We can do the
>> training validation on real HW when ready.
> 
> What information about the flash do you need for this training? 

Last time I looked, we lacked some post_init handler to setup a slave:
configure the registers defining the AHB windows for each flash
slave and perform the read timing calibration. calibration should
only be done once.

See how the aspeed_spi_flash_init() routine doing the calibration
is hooked up under aspeed_spi_claim_bus() in the u-boot driver :

   https://github.com/openbmc/u-boot/blob/v2019.04-aspeed-openbmc/drivers/spi/aspeed_spi.c

Not good enough for upstream, Linux would be the same :/

> I proposed a patch series [0] some time ago trying to implement training
> for TI SoCs. It did not get merged but I do intend to respin it and get
> it through. Would this API work for your tuning as well?

I will take a look.
  
> Also, I am curious how your training works. What data do you read for
> training delays? Where is it stored? 

The driver reads the first 16K at slow speed (that's why we need a
basic minimal setup of the slave) and checks if the buffer is valid
enough for the calibration :

   https://github.com/openbmc/linux/blob/dev-5.15/drivers/mtd/spi-nor/controllers/aspeed-smc.c#L998

it then performs reads by changing the frequency and delays and
compares results with the initial default buffer.

if not, then the driver stays in a safe mode (slow).

> In our case we need to flash a
> known pattern at some location (which is passed in via DT). Do you need
> to run it for every read transaction or just once after the flash is
> initialized?

Just once because it is a heavy process. See the debug outputs below.
Once we have good read timings and frequency, there is no need to do
it each time.

> [0] https://patchwork.ozlabs.org/project/linux-mtd/list/?series=233504&state=%2A&archive=both
Thanks,

C.



There are 3 controllers, 1e620000/FMC is for the BMC. We keep
safe settings for it and normally u-boot has done the training
already . The other two controllers are for the SPI-NOR of the
host and for these we push the frequency higher.


AST2600 EVB:

[    0.689662] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
[    0.696412] aspeed-smc 1e620000.spi: control register: 203b0641
[    0.696426] aspeed-smc 1e620000.spi: control register changed to: 00000600
[    0.696434] aspeed-smc 1e620000.spi: default control register: 00000600
[    0.696616] aspeed-smc 1e620000.spi: w25q512jv (65536 Kbytes)
[    0.703108] aspeed-smc 1e620000.spi: CE0 window [ 0x20000000 - 0x24000000 ] 64MB
[    0.711445] aspeed-smc 1e620000.spi: CE1 window [ 0x24000000 - 0x2c000000 ] 128MB
[    0.719864] aspeed-smc 1e620000.spi: write control register: 00120602
[    0.719873] aspeed-smc 1e620000.spi: read control register: 203c0641
[    0.727026] aspeed-smc 1e620000.spi: AHB frequency: 187 MHz
[    0.739247] aspeed-smc 1e620000.spi: Trying HCLK/5 [203c0d41] ...
[    0.767181] aspeed-smc 1e620000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    0.767196] aspeed-smc 1e620000.spi: Trying HCLK/4 [203c0641] ...
[    0.791559] aspeed-smc 1e620000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    0.791571] aspeed-smc 1e620000.spi: Found good read timings at HCLK/4
[    0.795729] 5 fixed-partitions partitions found on MTD device bmc
[    0.802636] Creating 5 MTD partitions on "bmc":
[    0.807739] 0x000000000000-0x0000000e0000 : "u-boot"
[    0.814367] 0x0000000e0000-0x000000100000 : "u-boot-env"
[    0.821306] 0x000000100000-0x000000a00000 : "kernel"
[    0.827755] 0x000000a00000-0x000002a00000 : "rofs"
[    0.834051] 0x000002a00000-0x000004000000 : "rwfs"
[    0.844040] aspeed-smc 1e630000.spi: Using 100 MHz SPI frequency
[    0.850912] aspeed-smc 1e630000.spi: control register: 00000400
[    0.850927] aspeed-smc 1e630000.spi: default control register: 00000400
[    0.851152] aspeed-smc 1e630000.spi: w25q256 (32768 Kbytes)
[    0.857427] aspeed-smc 1e630000.spi: CE0 window [ 0x30000000 - 0x32000000 ] 32MB
[    0.865792] aspeed-smc 1e630000.spi: CE1 window [ 0x32000000 - 0x32000000 ] 0MB (disabled)
[    0.875129] aspeed-smc 1e630000.spi: write control register: 00120402
[    0.875142] aspeed-smc 1e630000.spi: read control register: 203c0441
[    0.882296] aspeed-smc 1e630000.spi: AHB frequency: 187 MHz
[    0.894509] aspeed-smc 1e630000.spi: Trying HCLK/5 [203c0d41] ...
[    0.922417] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    0.922432] aspeed-smc 1e630000.spi: Trying HCLK/4 [203c0641] ...
[    0.946791] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    0.946803] aspeed-smc 1e630000.spi: Trying HCLK/3 [203c0e41] ...
[    0.967644] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    0.967655] aspeed-smc 1e630000.spi: Trying HCLK/2 [203c0741] ...
[    0.969325] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, DI delay none : FAIL
[    0.971007] aspeed-smc 1e630000.spi:   * [00000008] 0 HCLK delay, DI delay 0.5ns : FAIL
[    0.972679] aspeed-smc 1e630000.spi:   * [00000018] 0 HCLK delay, DI delay 1.5ns : FAIL
[    0.974350] aspeed-smc 1e630000.spi:   * [00000028] 0 HCLK delay, DI delay 1.5ns : FAIL
[    0.976021] aspeed-smc 1e630000.spi:   * [00000038] 0 HCLK delay, DI delay 2.5ns : FAIL
[    0.977692] aspeed-smc 1e630000.spi:   * [00000048] 0 HCLK delay, DI delay 2.5ns : FAIL
[    0.979363] aspeed-smc 1e630000.spi:   * [00000058] 0 HCLK delay, DI delay 3.5ns : FAIL
[    0.981042] aspeed-smc 1e630000.spi:   * [00000068] 0 HCLK delay, DI delay 3.5ns : FAIL
[    0.982714] aspeed-smc 1e630000.spi:   * [00000078] 0 HCLK delay, DI delay 4.5ns : FAIL
[    0.984385] aspeed-smc 1e630000.spi:   * [00000088] 0 HCLK delay, DI delay 4.5ns : FAIL
[    0.986056] aspeed-smc 1e630000.spi:   * [00000098] 0 HCLK delay, DI delay 5.5ns : FAIL
[    0.987727] aspeed-smc 1e630000.spi:   * [000000a8] 0 HCLK delay, DI delay 5.5ns : FAIL
[    0.989397] aspeed-smc 1e630000.spi:   * [000000b8] 0 HCLK delay, DI delay 6.5ns : FAIL
[    0.991084] aspeed-smc 1e630000.spi:   * [000000c8] 0 HCLK delay, DI delay 6.5ns : FAIL
[    0.992757] aspeed-smc 1e630000.spi:   * [000000d8] 0 HCLK delay, DI delay 7.5ns : FAIL
[    0.994428] aspeed-smc 1e630000.spi:   * [000000e8] 0 HCLK delay, DI delay 7.5ns : FAIL
[    0.996099] aspeed-smc 1e630000.spi:   * [000000f8] 0 HCLK delay, DI delay 8.5ns : FAIL
[    1.013874] aspeed-smc 1e630000.spi:   * [000000f1] 1 HCLK delay, DI delay none : PASS
[    1.013885] aspeed-smc 1e630000.spi: Found good read timings at HCLK/2
[    1.021498] aspeed-smc 1e631000.spi: Using 100 MHz SPI frequency
[    1.028291] aspeed-smc 1e631000.spi: control register: 00000400
[    1.028302] aspeed-smc 1e631000.spi: default control register: 00000400
[    1.028510] aspeed-smc 1e631000.spi: w25q256 (32768 Kbytes)
[    1.034848] aspeed-smc 1e631000.spi: CE0 window [ 0x50000000 - 0x52000000 ] 32MB
[    1.043197] aspeed-smc 1e631000.spi: CE1 window [ 0x52000000 - 0x52000000 ] 0MB (disabled)
[    1.052518] aspeed-smc 1e631000.spi: write control register: 00120402
[    1.052530] aspeed-smc 1e631000.spi: read control register: 203c0441
[    1.059677] aspeed-smc 1e631000.spi: AHB frequency: 187 MHz
[    1.071900] aspeed-smc 1e631000.spi: Trying HCLK/5 [203c0d41] ...
[    1.099805] aspeed-smc 1e631000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    1.099817] aspeed-smc 1e631000.spi: Trying HCLK/4 [203c0641] ...
[    1.124202] aspeed-smc 1e631000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    1.124219] aspeed-smc 1e631000.spi: Trying HCLK/3 [203c0e41] ...
[    1.145070] aspeed-smc 1e631000.spi:   * [00000000] 0 HCLK delay, DI delay none : PASS
[    1.145082] aspeed-smc 1e631000.spi: Trying HCLK/2 [203c0741] ...
[    1.146752] aspeed-smc 1e631000.spi:   * [00000000] 0 HCLK delay, DI delay none : FAIL
[    1.148422] aspeed-smc 1e631000.spi:   * [00000008] 0 HCLK delay, DI delay 0.5ns : FAIL
[    1.150093] aspeed-smc 1e631000.spi:   * [00000018] 0 HCLK delay, DI delay 1.5ns : FAIL
[    1.151778] aspeed-smc 1e631000.spi:   * [00000028] 0 HCLK delay, DI delay 1.5ns : FAIL
[    1.153451] aspeed-smc 1e631000.spi:   * [00000038] 0 HCLK delay, DI delay 2.5ns : FAIL
[    1.155122] aspeed-smc 1e631000.spi:   * [00000048] 0 HCLK delay, DI delay 2.5ns : FAIL
[    1.156793] aspeed-smc 1e631000.spi:   * [00000058] 0 HCLK delay, DI delay 3.5ns : FAIL
[    1.158464] aspeed-smc 1e631000.spi:   * [00000068] 0 HCLK delay, DI delay 3.5ns : FAIL
[    1.160135] aspeed-smc 1e631000.spi:   * [00000078] 0 HCLK delay, DI delay 4.5ns : FAIL
[    1.161818] aspeed-smc 1e631000.spi:   * [00000088] 0 HCLK delay, DI delay 4.5ns : FAIL
[    1.163490] aspeed-smc 1e631000.spi:   * [00000098] 0 HCLK delay, DI delay 5.5ns : FAIL
[    1.165161] aspeed-smc 1e631000.spi:   * [000000a8] 0 HCLK delay, DI delay 5.5ns : FAIL
[    1.166833] aspeed-smc 1e631000.spi:   * [000000b8] 0 HCLK delay, DI delay 6.5ns : FAIL
[    1.168504] aspeed-smc 1e631000.spi:   * [000000c8] 0 HCLK delay, DI delay 6.5ns : FAIL
[    1.170175] aspeed-smc 1e631000.spi:   * [000000d8] 0 HCLK delay, DI delay 7.5ns : FAIL
[    1.171863] aspeed-smc 1e631000.spi:   * [000000e8] 0 HCLK delay, DI delay 7.5ns : FAIL
[    1.173536] aspeed-smc 1e631000.spi:   * [000000f8] 0 HCLK delay, DI delay 8.5ns : FAIL
[    1.191318] aspeed-smc 1e631000.spi:   * [000000f1] 1 HCLK delay, DI delay none : PASS
[    1.191330] aspeed-smc 1e631000.spi: Found good read timings at HCLK/2


an ASTS2500 EVB :

[    1.220804] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency
[    1.226797] aspeed-smc 1e620000.spi: control register: 000b0641
[    1.226836] aspeed-smc 1e620000.spi: control register changed to: 00000600
[    1.226860] aspeed-smc 1e620000.spi: default control register: 00000600
[    1.227092] aspeed-smc 1e620000.spi: w25q256 (32768 Kbytes)
[    1.232806] aspeed-smc 1e620000.spi: CE0 window [ 0x20000000 - 0x22000000 ] 32MB
[    1.240329] aspeed-smc 1e620000.spi: CE1 window [ 0x22000000 - 0x2a000000 ] 128MB
[    1.247852] aspeed-smc 1e620000.spi: write control register: 00020602
[    1.247882] aspeed-smc 1e620000.spi: read control register: 203b0641
[    1.254315] aspeed-smc 1e620000.spi: AHB frequency: 198 MHz
[    1.265406] aspeed-smc 1e620000.spi: Trying HCLK/5 [203b0d41] ...
[    1.287111] aspeed-smc 1e620000.spi:   * [00080000] 0 HCLK delay, 4ns DI delay : PASS
[    1.309048] aspeed-smc 1e620000.spi:   * [00000000] 0 HCLK delay, 0ns DI delay : PASS
[    1.331223] aspeed-smc 1e620000.spi:   * [00090000] 1 HCLK delay, 4ns DI delay : PASS
[    1.331278] aspeed-smc 1e620000.spi:  * -> good is pass 1 [0x00000000]
[    1.331308] aspeed-smc 1e620000.spi: Trying HCLK/4 [203b0641] ...
[    1.349958] aspeed-smc 1e620000.spi:   * [00008000] 0 HCLK delay, 4ns DI delay : PASS
[    1.368473] aspeed-smc 1e620000.spi:   * [00000000] 0 HCLK delay, 0ns DI delay : PASS
[    1.387341] aspeed-smc 1e620000.spi:   * [00009000] 1 HCLK delay, 4ns DI delay : PASS
[    1.387397] aspeed-smc 1e620000.spi:  * -> good is pass 1 [0x00000000]
[    1.387435] aspeed-smc 1e620000.spi: Found good read timings at HCLK/4
[    1.858947] Freeing initrd memory: 1044K
[    1.906913] 5 fixed-partitions partitions found on MTD device bmc
[    1.913143] Creating 5 MTD partitions on "bmc":
[    1.917724] 0x000000000000-0x000000060000 : "u-boot"
[    1.925920] 0x000000060000-0x000000080000 : "u-boot-env"
[    1.937262] 0x000000080000-0x0000004c0000 : "kernel"
[    1.948189] 0x0000004c0000-0x000001c00000 : "rofs"
[    1.959196] 0x000001c00000-0x000002000000 : "rwfs"
[    1.971557] aspeed-smc 1e630000.spi: Using 100 MHz SPI frequency
[    1.977632] aspeed-smc 1e630000.spi: control register: 00000200
[    1.977669] aspeed-smc 1e630000.spi: default control register: 00000200
[    1.977961] aspeed-smc 1e630000.spi: w25q256 (32768 Kbytes)
[    1.983674] aspeed-smc 1e630000.spi: CE0 window [ 0x30000000 - 0x32000000 ] 32MB
[    1.991183] aspeed-smc 1e630000.spi: CE1 window [ 0x32000000 - 0x38000000 ] 96MB
[    1.998621] aspeed-smc 1e630000.spi: write control register: 00020202
[    1.998652] aspeed-smc 1e630000.spi: read control register: 203b0241
[    2.005086] aspeed-smc 1e630000.spi: AHB frequency: 198 MHz
[    2.016174] aspeed-smc 1e630000.spi: Trying HCLK/5 [203b0d41] ...
[    2.038011] aspeed-smc 1e630000.spi:   * [00080000] 0 HCLK delay, 4ns DI delay : PASS
[    2.060035] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, 0ns DI delay : PASS
[    2.082211] aspeed-smc 1e630000.spi:   * [00090000] 1 HCLK delay, 4ns DI delay : PASS
[    2.082266] aspeed-smc 1e630000.spi:  * -> good is pass 1 [0x00000000]
[    2.082295] aspeed-smc 1e630000.spi: Trying HCLK/4 [203b0641] ...
[    2.100938] aspeed-smc 1e630000.spi:   * [00008000] 0 HCLK delay, 4ns DI delay : PASS
[    2.119623] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, 0ns DI delay : PASS
[    2.138440] aspeed-smc 1e630000.spi:   * [00009000] 1 HCLK delay, 4ns DI delay : PASS
[    2.138491] aspeed-smc 1e630000.spi:  * -> good is pass 1 [0x00000000]
[    2.138521] aspeed-smc 1e630000.spi: Trying HCLK/3 [203b0e41] ...
[    2.139827] aspeed-smc 1e630000.spi:   * [00000800] 0 HCLK delay, 4ns DI delay : FAIL
[    2.155093] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, 0ns DI delay : PASS
[    2.170627] aspeed-smc 1e630000.spi:   * [00000900] 1 HCLK delay, 4ns DI delay : PASS
[    2.186111] aspeed-smc 1e630000.spi:   * [00000100] 1 HCLK delay, 0ns DI delay : PASS
[    2.186164] aspeed-smc 1e630000.spi:  * -> good is pass 2 [0x00000900]
[    2.186195] aspeed-smc 1e630000.spi: Trying HCLK/2 [203b0741] ...
[    2.187103] aspeed-smc 1e630000.spi:   * [00000080] 0 HCLK delay, 4ns DI delay : FAIL
[    2.188010] aspeed-smc 1e630000.spi:   * [00000000] 0 HCLK delay, 0ns DI delay : FAIL
[    2.200197] aspeed-smc 1e630000.spi:   * [00000090] 1 HCLK delay, 4ns DI delay : PASS
[    2.212359] aspeed-smc 1e630000.spi:   * [00000010] 1 HCLK delay, 0ns DI delay : PASS
[    2.224725] aspeed-smc 1e630000.spi:   * [000000a0] 2 HCLK delay, 4ns DI delay : PASS
[    2.224777] aspeed-smc 1e630000.spi:  * -> good is pass 3 [0x00000010]
[    2.224810] aspeed-smc 1e630000.spi: Found good read timings at HCLK/2
[    2.244098] aspeed-smc 1e631000.spi: Aspeed SMC probe failed -19

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Pratyush Yadav]

On 24/01/22 07:34PM, Cédric Le Goater wrote:
> > > spimem needs an extension I think. Sorry I have not been able to
> > > push that forward. Lack of time and other tasks to address on the
> > > host side of the machine. This is really a software problem, we
> > > have the HW procedures ready. If a spimem expert could get involved
> > > to make a few proposals, I would be happy to help and do some testing.
> > > QEMU models are good enough for the software part. We can do the
> > > training validation on real HW when ready.
> > 
> > What information about the flash do you need for this training?
> 
> Last time I looked, we lacked some post_init handler to setup a slave:
> configure the registers defining the AHB windows for each flash
> slave and perform the read timing calibration. calibration should
> only be done once.
> 
> See how the aspeed_spi_flash_init() routine doing the calibration
> is hooked up under aspeed_spi_claim_bus() in the u-boot driver :

My patch series should provide a hook for doing the calibration _after_ 
the flash is initialized.

> 
>   https://github.com/openbmc/u-boot/blob/v2019.04-aspeed-openbmc/drivers/spi/aspeed_spi.c
> 
> Not good enough for upstream, Linux would be the same :/
> 
> > I proposed a patch series [0] some time ago trying to implement training
> > for TI SoCs. It did not get merged but I do intend to respin it and get
> > it through. Would this API work for your tuning as well?
> 
> I will take a look.
> > Also, I am curious how your training works. What data do you read for
> > training delays? Where is it stored?
> 
> The driver reads the first 16K at slow speed (that's why we need a
> basic minimal setup of the slave) and checks if the buffer is valid
> enough for the calibration :
> 
>   https://github.com/openbmc/linux/blob/dev-5.15/drivers/mtd/spi-nor/controllers/aspeed-smc.c#L998
> 
> it then performs reads by changing the frequency and delays and
> compares results with the initial default buffer.

This seems similar to the tuning I implemented, except mine uses a 
pre-defined pattern at a pre-defined location.

> 
> if not, then the driver stays in a safe mode (slow).

Same for my patches.

> 
> > In our case we need to flash a
> > known pattern at some location (which is passed in via DT). Do you need
> > to run it for every read transaction or just once after the flash is
> > initialized?
> 
> Just once because it is a heavy process. See the debug outputs below.
> Once we have good read timings and frequency, there is no need to do
> it each time.

It looks very similar to the tuning I implemented in my patch series. 
You should be able to use those APIs for your tuning as well. But it 
should not block the SPI MEM port. The current upstream driver does not 
seem to implement this tuning anyway.

> 
> > [0] https://patchwork.ozlabs.org/project/linux-mtd/list/?series=233504&state=%2A&archive=both
> Thanks,
> 
> C.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Pratyush Yadav]

On 24/01/22 07:34PM, Cédric Le Goater wrote:
> > > spimem needs an extension I think. Sorry I have not been able to
> > > push that forward. Lack of time and other tasks to address on the
> > > host side of the machine. This is really a software problem, we
> > > have the HW procedures ready. If a spimem expert could get involved
> > > to make a few proposals, I would be happy to help and do some testing.
> > > QEMU models are good enough for the software part. We can do the
> > > training validation on real HW when ready.
> > 
> > What information about the flash do you need for this training?
> 
> Last time I looked, we lacked some post_init handler to setup a slave:
> configure the registers defining the AHB windows for each flash
> slave and perform the read timing calibration. calibration should
> only be done once.
> 
> See how the aspeed_spi_flash_init() routine doing the calibration
> is hooked up under aspeed_spi_claim_bus() in the u-boot driver :

My patch series should provide a hook for doing the calibration _after_ 
the flash is initialized.

> 
>   https://github.com/openbmc/u-boot/blob/v2019.04-aspeed-openbmc/drivers/spi/aspeed_spi.c
> 
> Not good enough for upstream, Linux would be the same :/
> 
> > I proposed a patch series [0] some time ago trying to implement training
> > for TI SoCs. It did not get merged but I do intend to respin it and get
> > it through. Would this API work for your tuning as well?
> 
> I will take a look.
> > Also, I am curious how your training works. What data do you read for
> > training delays? Where is it stored?
> 
> The driver reads the first 16K at slow speed (that's why we need a
> basic minimal setup of the slave) and checks if the buffer is valid
> enough for the calibration :
> 
>   https://github.com/openbmc/linux/blob/dev-5.15/drivers/mtd/spi-nor/controllers/aspeed-smc.c#L998
> 
> it then performs reads by changing the frequency and delays and
> compares results with the initial default buffer.

This seems similar to the tuning I implemented, except mine uses a 
pre-defined pattern at a pre-defined location.

> 
> if not, then the driver stays in a safe mode (slow).

Same for my patches.

> 
> > In our case we need to flash a
> > known pattern at some location (which is passed in via DT). Do you need
> > to run it for every read transaction or just once after the flash is
> > initialized?
> 
> Just once because it is a heavy process. See the debug outputs below.
> Once we have good read timings and frequency, there is no need to do
> it each time.

It looks very similar to the tuning I implemented in my patch series. 
You should be able to use those APIs for your tuning as well. But it 
should not block the SPI MEM port. The current upstream driver does not 
seem to implement this tuning anyway.

> 
> > [0] https://patchwork.ozlabs.org/project/linux-mtd/list/?series=233504&state=%2A&archive=both
> Thanks,
> 
> C.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Pratyush Yadav]

On 24/01/22 07:34PM, Cédric Le Goater wrote:
> > > spimem needs an extension I think. Sorry I have not been able to
> > > push that forward. Lack of time and other tasks to address on the
> > > host side of the machine. This is really a software problem, we
> > > have the HW procedures ready. If a spimem expert could get involved
> > > to make a few proposals, I would be happy to help and do some testing.
> > > QEMU models are good enough for the software part. We can do the
> > > training validation on real HW when ready.
> > 
> > What information about the flash do you need for this training?
> 
> Last time I looked, we lacked some post_init handler to setup a slave:
> configure the registers defining the AHB windows for each flash
> slave and perform the read timing calibration. calibration should
> only be done once.
> 
> See how the aspeed_spi_flash_init() routine doing the calibration
> is hooked up under aspeed_spi_claim_bus() in the u-boot driver :

My patch series should provide a hook for doing the calibration _after_ 
the flash is initialized.

> 
>   https://github.com/openbmc/u-boot/blob/v2019.04-aspeed-openbmc/drivers/spi/aspeed_spi.c
> 
> Not good enough for upstream, Linux would be the same :/
> 
> > I proposed a patch series [0] some time ago trying to implement training
> > for TI SoCs. It did not get merged but I do intend to respin it and get
> > it through. Would this API work for your tuning as well?
> 
> I will take a look.
> > Also, I am curious how your training works. What data do you read for
> > training delays? Where is it stored?
> 
> The driver reads the first 16K at slow speed (that's why we need a
> basic minimal setup of the slave) and checks if the buffer is valid
> enough for the calibration :
> 
>   https://github.com/openbmc/linux/blob/dev-5.15/drivers/mtd/spi-nor/controllers/aspeed-smc.c#L998
> 
> it then performs reads by changing the frequency and delays and
> compares results with the initial default buffer.

This seems similar to the tuning I implemented, except mine uses a 
pre-defined pattern at a pre-defined location.

> 
> if not, then the driver stays in a safe mode (slow).

Same for my patches.

> 
> > In our case we need to flash a
> > known pattern at some location (which is passed in via DT). Do you need
> > to run it for every read transaction or just once after the flash is
> > initialized?
> 
> Just once because it is a heavy process. See the debug outputs below.
> Once we have good read timings and frequency, there is no need to do
> it each time.

It looks very similar to the tuning I implemented in my patch series. 
You should be able to use those APIs for your tuning as well. But it 
should not block the SPI MEM port. The current upstream driver does not 
seem to implement this tuning anyway.

> 
> > [0] https://patchwork.ozlabs.org/project/linux-mtd/list/?series=233504&state=%2A&archive=both
> Thanks,
> 
> C.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Cédric Le Goater]

Hello,

On 1/24/22 21:37, Pratyush Yadav wrote:
> On 24/01/22 07:34PM, Cédric Le Goater wrote:
>>>> spimem needs an extension I think. Sorry I have not been able to
>>>> push that forward. Lack of time and other tasks to address on the
>>>> host side of the machine. This is really a software problem, we
>>>> have the HW procedures ready. If a spimem expert could get involved
>>>> to make a few proposals, I would be happy to help and do some testing.
>>>> QEMU models are good enough for the software part. We can do the
>>>> training validation on real HW when ready.
>>>
>>> What information about the flash do you need for this training?
>>
>> Last time I looked, we lacked some post_init handler to setup a slave:
>> configure the registers defining the AHB windows for each flash
>> slave and perform the read timing calibration. calibration should
>> only be done once.
>>
>> See how the aspeed_spi_flash_init() routine doing the calibration
>> is hooked up under aspeed_spi_claim_bus() in the u-boot driver :
> 
> My patch series should provide a hook for doing the calibration _after_
> the flash is initialized.

You can also use the .dirmap_create handler. The flash device has
been scanned when called and the size is available in the spi-mem
dirmap descriptor.

I reworked the current Aspeed driver with this approach and it
seems sufficient for read calibration.

Thanks,

C.


> 
>>
>>    https://github.com/openbmc/u-boot/blob/v2019.04-aspeed-openbmc/drivers/spi/aspeed_spi.c
>>
>> Not good enough for upstream, Linux would be the same :/
>>
>>> I proposed a patch series [0] some time ago trying to implement training
>>> for TI SoCs. It did not get merged but I do intend to respin it and get
>>> it through. Would this API work for your tuning as well?
>>
>> I will take a look.
>>> Also, I am curious how your training works. What data do you read for
>>> training delays? Where is it stored?
>>
>> The driver reads the first 16K at slow speed (that's why we need a
>> basic minimal setup of the slave) and checks if the buffer is valid
>> enough for the calibration :
>>
>>    https://github.com/openbmc/linux/blob/dev-5.15/drivers/mtd/spi-nor/controllers/aspeed-smc.c#L998
>>
>> it then performs reads by changing the frequency and delays and
>> compares results with the initial default buffer.
> 
> This seems similar to the tuning I implemented, except mine uses a
> pre-defined pattern at a pre-defined location.
> 
>>
>> if not, then the driver stays in a safe mode (slow).
> 
> Same for my patches.
> 
>>
>>> In our case we need to flash a
>>> known pattern at some location (which is passed in via DT). Do you need
>>> to run it for every read transaction or just once after the flash is
>>> initialized?
>>
>> Just once because it is a heavy process. See the debug outputs below.
>> Once we have good read timings and frequency, there is no need to do
>> it each time.
> 
> It looks very similar to the tuning I implemented in my patch series.
> You should be able to use those APIs for your tuning as well. But it
> should not block the SPI MEM port. The current upstream driver does not
> seem to implement this tuning anyway.
> 
>>
>>> [0] https://patchwork.ozlabs.org/project/linux-mtd/list/?series=233504&state=%2A&archive=both
>> Thanks,
>>
>> C.
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Cédric Le Goater]

Hello,

On 1/24/22 21:37, Pratyush Yadav wrote:
> On 24/01/22 07:34PM, Cédric Le Goater wrote:
>>>> spimem needs an extension I think. Sorry I have not been able to
>>>> push that forward. Lack of time and other tasks to address on the
>>>> host side of the machine. This is really a software problem, we
>>>> have the HW procedures ready. If a spimem expert could get involved
>>>> to make a few proposals, I would be happy to help and do some testing.
>>>> QEMU models are good enough for the software part. We can do the
>>>> training validation on real HW when ready.
>>>
>>> What information about the flash do you need for this training?
>>
>> Last time I looked, we lacked some post_init handler to setup a slave:
>> configure the registers defining the AHB windows for each flash
>> slave and perform the read timing calibration. calibration should
>> only be done once.
>>
>> See how the aspeed_spi_flash_init() routine doing the calibration
>> is hooked up under aspeed_spi_claim_bus() in the u-boot driver :
> 
> My patch series should provide a hook for doing the calibration _after_
> the flash is initialized.

You can also use the .dirmap_create handler. The flash device has
been scanned when called and the size is available in the spi-mem
dirmap descriptor.

I reworked the current Aspeed driver with this approach and it
seems sufficient for read calibration.

Thanks,

C.


> 
>>
>>    https://github.com/openbmc/u-boot/blob/v2019.04-aspeed-openbmc/drivers/spi/aspeed_spi.c
>>
>> Not good enough for upstream, Linux would be the same :/
>>
>>> I proposed a patch series [0] some time ago trying to implement training
>>> for TI SoCs. It did not get merged but I do intend to respin it and get
>>> it through. Would this API work for your tuning as well?
>>
>> I will take a look.
>>> Also, I am curious how your training works. What data do you read for
>>> training delays? Where is it stored?
>>
>> The driver reads the first 16K at slow speed (that's why we need a
>> basic minimal setup of the slave) and checks if the buffer is valid
>> enough for the calibration :
>>
>>    https://github.com/openbmc/linux/blob/dev-5.15/drivers/mtd/spi-nor/controllers/aspeed-smc.c#L998
>>
>> it then performs reads by changing the frequency and delays and
>> compares results with the initial default buffer.
> 
> This seems similar to the tuning I implemented, except mine uses a
> pre-defined pattern at a pre-defined location.
> 
>>
>> if not, then the driver stays in a safe mode (slow).
> 
> Same for my patches.
> 
>>
>>> In our case we need to flash a
>>> known pattern at some location (which is passed in via DT). Do you need
>>> to run it for every read transaction or just once after the flash is
>>> initialized?
>>
>> Just once because it is a heavy process. See the debug outputs below.
>> Once we have good read timings and frequency, there is no need to do
>> it each time.
> 
> It looks very similar to the tuning I implemented in my patch series.
> You should be able to use those APIs for your tuning as well. But it
> should not block the SPI MEM port. The current upstream driver does not
> seem to implement this tuning anyway.
> 
>>
>>> [0] https://patchwork.ozlabs.org/project/linux-mtd/list/?series=233504&state=%2A&archive=both
>> Thanks,
>>
>> C.
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Cédric Le Goater]

Hello,

On 1/24/22 21:37, Pratyush Yadav wrote:
> On 24/01/22 07:34PM, Cédric Le Goater wrote:
>>>> spimem needs an extension I think. Sorry I have not been able to
>>>> push that forward. Lack of time and other tasks to address on the
>>>> host side of the machine. This is really a software problem, we
>>>> have the HW procedures ready. If a spimem expert could get involved
>>>> to make a few proposals, I would be happy to help and do some testing.
>>>> QEMU models are good enough for the software part. We can do the
>>>> training validation on real HW when ready.
>>>
>>> What information about the flash do you need for this training?
>>
>> Last time I looked, we lacked some post_init handler to setup a slave:
>> configure the registers defining the AHB windows for each flash
>> slave and perform the read timing calibration. calibration should
>> only be done once.
>>
>> See how the aspeed_spi_flash_init() routine doing the calibration
>> is hooked up under aspeed_spi_claim_bus() in the u-boot driver :
> 
> My patch series should provide a hook for doing the calibration _after_
> the flash is initialized.

You can also use the .dirmap_create handler. The flash device has
been scanned when called and the size is available in the spi-mem
dirmap descriptor.

I reworked the current Aspeed driver with this approach and it
seems sufficient for read calibration.

Thanks,

C.


> 
>>
>>    https://github.com/openbmc/u-boot/blob/v2019.04-aspeed-openbmc/drivers/spi/aspeed_spi.c
>>
>> Not good enough for upstream, Linux would be the same :/
>>
>>> I proposed a patch series [0] some time ago trying to implement training
>>> for TI SoCs. It did not get merged but I do intend to respin it and get
>>> it through. Would this API work for your tuning as well?
>>
>> I will take a look.
>>> Also, I am curious how your training works. What data do you read for
>>> training delays? Where is it stored?
>>
>> The driver reads the first 16K at slow speed (that's why we need a
>> basic minimal setup of the slave) and checks if the buffer is valid
>> enough for the calibration :
>>
>>    https://github.com/openbmc/linux/blob/dev-5.15/drivers/mtd/spi-nor/controllers/aspeed-smc.c#L998
>>
>> it then performs reads by changing the frequency and delays and
>> compares results with the initial default buffer.
> 
> This seems similar to the tuning I implemented, except mine uses a
> pre-defined pattern at a pre-defined location.
> 
>>
>> if not, then the driver stays in a safe mode (slow).
> 
> Same for my patches.
> 
>>
>>> In our case we need to flash a
>>> known pattern at some location (which is passed in via DT). Do you need
>>> to run it for every read transaction or just once after the flash is
>>> initialized?
>>
>> Just once because it is a heavy process. See the debug outputs below.
>> Once we have good read timings and frequency, there is no need to do
>> it each time.
> 
> It looks very similar to the tuning I implemented in my patch series.
> You should be able to use those APIs for your tuning as well. But it
> should not block the SPI MEM port. The current upstream driver does not
> seem to implement this tuning anyway.
> 
>>
>>> [0] https://patchwork.ozlabs.org/project/linux-mtd/list/?series=233504&state=%2A&archive=both
>> Thanks,
>>
>> C.
> 

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Pratyush Yadav]

On 07/02/22 06:13PM, Cédric Le Goater wrote:
> Hello,
> 
> On 1/24/22 21:37, Pratyush Yadav wrote:
> > On 24/01/22 07:34PM, Cédric Le Goater wrote:
> > > > > spimem needs an extension I think. Sorry I have not been able to
> > > > > push that forward. Lack of time and other tasks to address on the
> > > > > host side of the machine. This is really a software problem, we
> > > > > have the HW procedures ready. If a spimem expert could get involved
> > > > > to make a few proposals, I would be happy to help and do some testing.
> > > > > QEMU models are good enough for the software part. We can do the
> > > > > training validation on real HW when ready.
> > > > 
> > > > What information about the flash do you need for this training?
> > > 
> > > Last time I looked, we lacked some post_init handler to setup a slave:
> > > configure the registers defining the AHB windows for each flash
> > > slave and perform the read timing calibration. calibration should
> > > only be done once.
> > > 
> > > See how the aspeed_spi_flash_init() routine doing the calibration
> > > is hooked up under aspeed_spi_claim_bus() in the u-boot driver :
> > 
> > My patch series should provide a hook for doing the calibration _after_
> > the flash is initialized.
> 
> You can also use the .dirmap_create handler. The flash device has
> been scanned when called and the size is available in the spi-mem
> dirmap descriptor.

I feel uncomfortable doing that since the API does not actually make 
this guarantee. Who knows if a future change will violate that 
assumption. That is why I added a new API call to explicitly mark the 
flash as ready. I suppose you can get the op from the .dirmap_create 
handler, but I guess we can debate that over the patches.

> 
> I reworked the current Aspeed driver with this approach and it
> seems sufficient for read calibration.
> 
> Thanks,
> 
> C.
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Pratyush Yadav]

On 07/02/22 06:13PM, Cédric Le Goater wrote:
> Hello,
> 
> On 1/24/22 21:37, Pratyush Yadav wrote:
> > On 24/01/22 07:34PM, Cédric Le Goater wrote:
> > > > > spimem needs an extension I think. Sorry I have not been able to
> > > > > push that forward. Lack of time and other tasks to address on the
> > > > > host side of the machine. This is really a software problem, we
> > > > > have the HW procedures ready. If a spimem expert could get involved
> > > > > to make a few proposals, I would be happy to help and do some testing.
> > > > > QEMU models are good enough for the software part. We can do the
> > > > > training validation on real HW when ready.
> > > > 
> > > > What information about the flash do you need for this training?
> > > 
> > > Last time I looked, we lacked some post_init handler to setup a slave:
> > > configure the registers defining the AHB windows for each flash
> > > slave and perform the read timing calibration. calibration should
> > > only be done once.
> > > 
> > > See how the aspeed_spi_flash_init() routine doing the calibration
> > > is hooked up under aspeed_spi_claim_bus() in the u-boot driver :
> > 
> > My patch series should provide a hook for doing the calibration _after_
> > the flash is initialized.
> 
> You can also use the .dirmap_create handler. The flash device has
> been scanned when called and the size is available in the spi-mem
> dirmap descriptor.

I feel uncomfortable doing that since the API does not actually make 
this guarantee. Who knows if a future change will violate that 
assumption. That is why I added a new API call to explicitly mark the 
flash as ready. I suppose you can get the op from the .dirmap_create 
handler, but I guess we can debate that over the patches.

> 
> I reworked the current Aspeed driver with this approach and it
> seems sufficient for read calibration.
> 
> Thanks,
> 
> C.
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mtd: aspeed-smc: improve probe resilience

[Pratyush Yadav]

On 07/02/22 06:13PM, Cédric Le Goater wrote:
> Hello,
> 
> On 1/24/22 21:37, Pratyush Yadav wrote:
> > On 24/01/22 07:34PM, Cédric Le Goater wrote:
> > > > > spimem needs an extension I think. Sorry I have not been able to
> > > > > push that forward. Lack of time and other tasks to address on the
> > > > > host side of the machine. This is really a software problem, we
> > > > > have the HW procedures ready. If a spimem expert could get involved
> > > > > to make a few proposals, I would be happy to help and do some testing.
> > > > > QEMU models are good enough for the software part. We can do the
> > > > > training validation on real HW when ready.
> > > > 
> > > > What information about the flash do you need for this training?
> > > 
> > > Last time I looked, we lacked some post_init handler to setup a slave:
> > > configure the registers defining the AHB windows for each flash
> > > slave and perform the read timing calibration. calibration should
> > > only be done once.
> > > 
> > > See how the aspeed_spi_flash_init() routine doing the calibration
> > > is hooked up under aspeed_spi_claim_bus() in the u-boot driver :
> > 
> > My patch series should provide a hook for doing the calibration _after_
> > the flash is initialized.
> 
> You can also use the .dirmap_create handler. The flash device has
> been scanned when called and the size is available in the spi-mem
> dirmap descriptor.

I feel uncomfortable doing that since the API does not actually make 
this guarantee. Who knows if a future change will violate that 
assumption. That is why I added a new API call to explicitly mark the 
flash as ready. I suppose you can get the op from the .dirmap_create 
handler, but I guess we can debate that over the patches.

> 
> I reworked the current Aspeed driver with this approach and it
> seems sufficient for read calibration.
> 
> Thanks,
> 
> C.
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.