mirror of
https://github.com/coolsnowwolf/lede.git
synced 2025-06-13 23:42:04 +08:00
175 lines
6.7 KiB
Diff
175 lines
6.7 KiB
Diff
From: Gabor Juhos <j4g8y7@gmail.com>
|
|
Date: Thu, 01 May 2025 18:19:16 +0200
|
|
Subject: [PATCH next] spi: spi-qpic-snand: validate user/chip specific ECC
|
|
properties
|
|
|
|
The driver only supports 512 bytes ECC step size and 4 bit ECC strength
|
|
at the moment, however it does not reject unsupported step/strength
|
|
configurations. Due to this, whenever the driver is used with a flash
|
|
chip which needs stronger ECC protection, the following warning is shown
|
|
in the kernel log:
|
|
|
|
[ 0.574648] spi-nand spi0.0: GigaDevice SPI NAND was found.
|
|
[ 0.635748] spi-nand spi0.0: 256 MiB, block size: 128 KiB, page size: 2048, OOB size: 128
|
|
[ 0.649079] nand: WARNING: (null): the ECC used on your system is too weak compared to the one required by the NAND chip
|
|
|
|
Although the message indicates that something is wrong, but it often gets
|
|
unnoticed, which can cause serious problems. For example when the user
|
|
writes something into the flash chip despite the warning, the written data
|
|
may won't be readable by the bootloader or by the boot ROM. In the worst
|
|
case, when the attached SPI NAND chip is the boot device, the board may not
|
|
be able to boot anymore.
|
|
|
|
Also, it is not even possible to create a backup of the flash, because
|
|
reading its content results in bogus data. For example, dumping the first
|
|
page of the flash gives this:
|
|
|
|
# hexdump -C -n 2048 /dev/mtd0
|
|
00000000 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f |................|
|
|
*
|
|
00000040 0f 0f 0f 0f 0f 0f 0f 0d 0f 0f 0f 0f 0f 0f 0f 0f |................|
|
|
00000050 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f |................|
|
|
*
|
|
000001c0 0f 0f 0f 0f ff 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f |................|
|
|
000001d0 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f |................|
|
|
*
|
|
00000200 0f 0f 0f 0f f5 5b ff ff 0f 0f 0f 0f 0f 0f 0f 0f |.....[..........|
|
|
00000210 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f |................|
|
|
*
|
|
000002f0 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 1f 0f 0f |................|
|
|
00000300 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f |................|
|
|
*
|
|
000003c0 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f ff 0f 0f 0f |................|
|
|
000003d0 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f |................|
|
|
*
|
|
00000400 0f 0f 0f 0f 0f 0f 0f 0f e9 74 c9 06 f5 5b ff ff |.........t...[..|
|
|
00000410 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f |................|
|
|
*
|
|
000005d0 0f 0f 0f 0f ff 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f |................|
|
|
000005e0 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f |................|
|
|
*
|
|
00000600 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f c6 be 0f c3 |................|
|
|
00000610 e9 74 c9 06 f5 5b ff ff 0f 0f 0f 0f 0f 0f 0f 0f |.t...[..........|
|
|
00000620 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f |................|
|
|
*
|
|
00000770 0f 0f 0f 0f 8f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f |................|
|
|
00000780 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f |................|
|
|
*
|
|
00000800
|
|
#
|
|
|
|
Doing the same by using the downstream kernel results in different output:
|
|
|
|
# hexdump -C -n 2048 /dev/mtd0
|
|
00000000 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f |................|
|
|
*
|
|
00000800
|
|
#
|
|
|
|
This patch adds some sanity checks to the code to prevent using the driver
|
|
with unsupported ECC step/strength configurations. After the change, probing
|
|
of the driver fails in such cases:
|
|
|
|
[ 0.655038] spi-nand spi0.0: GigaDevice SPI NAND was found.
|
|
[ 0.659159] spi-nand spi0.0: 256 MiB, block size: 128 KiB, page size: 2048, OOB size: 128
|
|
[ 0.669138] qcom_snand 79b0000.spi: only 4 bits ECC strength is supported
|
|
[ 0.677476] nand: No suitable ECC configuration
|
|
[ 0.689909] spi-nand spi0.0: probe with driver spi-nand failed with error -95
|
|
|
|
This helps to avoid the aforementioned hassles until support for 8 bit ECC
|
|
strength gets implemented.
|
|
|
|
Fixes: 7304d1909080 ("spi: spi-qpic: add driver for QCOM SPI NAND flash Interface")
|
|
Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
|
|
---
|
|
Marked for next because it depends on commit f48d80503504 ("spi: spi-qpic-snand:
|
|
use kmalloc() for OOB buffer allocation").
|
|
---
|
|
drivers/spi/spi-qpic-snand.c | 42 +++++++++++++++++++++++++++++++++++++-----
|
|
1 file changed, 37 insertions(+), 5 deletions(-)
|
|
|
|
|
|
---
|
|
base-commit: a7f035c2c72496cf7ac34bfaa8c289e0d4c45836
|
|
change-id: 20250501-qpic-snand-validate-ecc-383b3e33e238
|
|
|
|
Best regards,
|
|
|
|
--- a/drivers/spi/spi-qpic-snand.c
|
|
+++ b/drivers/spi/spi-qpic-snand.c
|
|
@@ -249,9 +249,11 @@ static const struct mtd_ooblayout_ops qc
|
|
static int qcom_spi_ecc_init_ctx_pipelined(struct nand_device *nand)
|
|
{
|
|
struct qcom_nand_controller *snandc = nand_to_qcom_snand(nand);
|
|
+ struct nand_ecc_props *reqs = &nand->ecc.requirements;
|
|
+ struct nand_ecc_props *user = &nand->ecc.user_conf;
|
|
struct nand_ecc_props *conf = &nand->ecc.ctx.conf;
|
|
struct mtd_info *mtd = nanddev_to_mtd(nand);
|
|
- int cwperpage, bad_block_byte;
|
|
+ int cwperpage, bad_block_byte, ret;
|
|
struct qpic_ecc *ecc_cfg;
|
|
|
|
cwperpage = mtd->writesize / NANDC_STEP_SIZE;
|
|
@@ -260,11 +262,39 @@ static int qcom_spi_ecc_init_ctx_pipelin
|
|
ecc_cfg = kzalloc(sizeof(*ecc_cfg), GFP_KERNEL);
|
|
if (!ecc_cfg)
|
|
return -ENOMEM;
|
|
+
|
|
+ if (user->step_size && user->strength) {
|
|
+ ecc_cfg->step_size = user->step_size;
|
|
+ ecc_cfg->strength = user->strength;
|
|
+ } else if (reqs->step_size && reqs->strength) {
|
|
+ ecc_cfg->step_size = reqs->step_size;
|
|
+ ecc_cfg->strength = reqs->strength;
|
|
+ } else {
|
|
+ /* use defaults */
|
|
+ ecc_cfg->step_size = NANDC_STEP_SIZE;
|
|
+ ecc_cfg->strength = 4;
|
|
+ }
|
|
+
|
|
+ if (ecc_cfg->step_size != NANDC_STEP_SIZE) {
|
|
+ dev_err(snandc->dev,
|
|
+ "only %u bytes ECC step size is supported\n",
|
|
+ NANDC_STEP_SIZE);
|
|
+ ret = -EOPNOTSUPP;
|
|
+ goto err_free_ecc_cfg;
|
|
+ }
|
|
+
|
|
+ if (ecc_cfg->strength != 4) {
|
|
+ dev_err(snandc->dev,
|
|
+ "only 4 bits ECC strength is supported\n");
|
|
+ ret = -EOPNOTSUPP;
|
|
+ goto err_free_ecc_cfg;
|
|
+ }
|
|
+
|
|
snandc->qspi->oob_buf = kmalloc(mtd->writesize + mtd->oobsize,
|
|
GFP_KERNEL);
|
|
if (!snandc->qspi->oob_buf) {
|
|
- kfree(ecc_cfg);
|
|
- return -ENOMEM;
|
|
+ ret = -ENOMEM;
|
|
+ goto err_free_ecc_cfg;
|
|
}
|
|
|
|
memset(snandc->qspi->oob_buf, 0xff, mtd->writesize + mtd->oobsize);
|
|
@@ -279,8 +309,6 @@ static int qcom_spi_ecc_init_ctx_pipelin
|
|
ecc_cfg->bytes = ecc_cfg->ecc_bytes_hw + ecc_cfg->spare_bytes + ecc_cfg->bbm_size;
|
|
|
|
ecc_cfg->steps = 4;
|
|
- ecc_cfg->strength = 4;
|
|
- ecc_cfg->step_size = 512;
|
|
ecc_cfg->cw_data = 516;
|
|
ecc_cfg->cw_size = ecc_cfg->cw_data + ecc_cfg->bytes;
|
|
bad_block_byte = mtd->writesize - ecc_cfg->cw_size * (cwperpage - 1) + 1;
|
|
@@ -338,6 +366,10 @@ static int qcom_spi_ecc_init_ctx_pipelin
|
|
ecc_cfg->strength, ecc_cfg->step_size);
|
|
|
|
return 0;
|
|
+
|
|
+err_free_ecc_cfg:
|
|
+ kfree(ecc_cfg);
|
|
+ return ret;
|
|
}
|
|
|
|
static void qcom_spi_ecc_cleanup_ctx_pipelined(struct nand_device *nand)
|