[PATCH v2] mtd: rawnand: qcom: fix broken config in qcom_param_page_type_exec

[PATCH v2] mtd: rawnand: qcom: fix broken config in qcom_param_page_type_exec

[Christian Marangi]

Fix broken config in qcom_param_page_type_exec caused by copy-paste error
from commit 0c08080fd71c ("mtd: rawnand: qcom: use FIELD_PREP and GENMASK")

In qcom_param_page_type_exec the value needs to be set to
nandc->regs->cfg0 instead of host->cfg0. This wrong configuration caused
the Qcom NANDC driver to malfunction on any device that makes use of it
(IPQ806x, IPQ40xx, IPQ807x, IPQ60xx) with the following error:

[    0.885369] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xaa
[    0.885909] nand: Micron NAND 256MiB 1,8V 8-bit
[    0.892499] nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
[    0.896823] nand: ECC (step, strength) = (512, 8) does not fit in OOB
[    0.896836] qcom-nandc 79b0000.nand-controller: No valid ECC settings possible
[    0.910996] bam-dma-engine 7984000.dma-controller: Cannot free busy channel
[    0.918070] qcom-nandc: probe of 79b0000.nand-controller failed with error -28

Restore original configuration fix the problem and makes the driver work
again.

Also restore the wrongly dropped cpu_to_le32 to correctly support BE
systems.

Cc: stable@vger.kernel.org
Fixes: 0c08080fd71c ("mtd: rawnand: qcom: use FIELD_PREP and GENMASK")
Tested-by: Robert Marko <robimarko@gmail.com> # IPQ8074 and IPQ6018
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
Changes v2:
- Fix smatch warning (add missing cpu_to_le32 that was also dropped
  from the FIELD_PREP patch)

 drivers/mtd/nand/raw/qcom_nandc.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index d2d2aeee42a7..6720b547892b 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -1881,18 +1881,18 @@ static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_
 	nandc->regs->addr0 = 0;
 	nandc->regs->addr1 = 0;
 
-	host->cfg0 = FIELD_PREP(CW_PER_PAGE_MASK, 0) |
-		     FIELD_PREP(UD_SIZE_BYTES_MASK, 512) |
-		     FIELD_PREP(NUM_ADDR_CYCLES_MASK, 5) |
-		     FIELD_PREP(SPARE_SIZE_BYTES_MASK, 0);
-
-	host->cfg1 = FIELD_PREP(NAND_RECOVERY_CYCLES_MASK, 7) |
-		     FIELD_PREP(BAD_BLOCK_BYTE_NUM_MASK, 17) |
-		     FIELD_PREP(CS_ACTIVE_BSY, 0) |
-		     FIELD_PREP(BAD_BLOCK_IN_SPARE_AREA, 1) |
-		     FIELD_PREP(WR_RD_BSY_GAP_MASK, 2) |
-		     FIELD_PREP(WIDE_FLASH, 0) |
-		     FIELD_PREP(DEV0_CFG1_ECC_DISABLE, 1);
+	nandc->regs->cfg0 = cpu_to_le32(FIELD_PREP(CW_PER_PAGE_MASK, 0) |
+					FIELD_PREP(UD_SIZE_BYTES_MASK, 512) |
+					FIELD_PREP(NUM_ADDR_CYCLES_MASK, 5) |
+					FIELD_PREP(SPARE_SIZE_BYTES_MASK, 0));
+
+	nandc->regs->cfg1 = cpu_to_le32(FIELD_PREP(NAND_RECOVERY_CYCLES_MASK, 7) |
+					FIELD_PREP(BAD_BLOCK_BYTE_NUM_MASK, 17) |
+					FIELD_PREP(CS_ACTIVE_BSY, 0) |
+					FIELD_PREP(BAD_BLOCK_IN_SPARE_AREA, 1) |
+					FIELD_PREP(WR_RD_BSY_GAP_MASK, 2) |
+					FIELD_PREP(WIDE_FLASH, 0) |
+					FIELD_PREP(DEV0_CFG1_ECC_DISABLE, 1));
 
 	if (!nandc->props->qpic_version2)
 		nandc->regs->ecc_buf_cfg = cpu_to_le32(ECC_CFG_ECC_DISABLE);
-- 
2.47.1


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

Re: [PATCH v2] mtd: rawnand: qcom: fix broken config in qcom_param_page_type_exec

[Manivannan Sadhasivam]

On Sun, Feb 09, 2025 at 03:09:38PM +0100, Christian Marangi wrote:
> Fix broken config in qcom_param_page_type_exec caused by copy-paste error
> from commit 0c08080fd71c ("mtd: rawnand: qcom: use FIELD_PREP and GENMASK")
> 
> In qcom_param_page_type_exec the value needs to be set to
> nandc->regs->cfg0 instead of host->cfg0. This wrong configuration caused
> the Qcom NANDC driver to malfunction on any device that makes use of it
> (IPQ806x, IPQ40xx, IPQ807x, IPQ60xx) with the following error:
> 

I'm wondering whether the offending commit was really tested or not :(

> [    0.885369] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xaa
> [    0.885909] nand: Micron NAND 256MiB 1,8V 8-bit
> [    0.892499] nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
> [    0.896823] nand: ECC (step, strength) = (512, 8) does not fit in OOB
> [    0.896836] qcom-nandc 79b0000.nand-controller: No valid ECC settings possible
> [    0.910996] bam-dma-engine 7984000.dma-controller: Cannot free busy channel
> [    0.918070] qcom-nandc: probe of 79b0000.nand-controller failed with error -28
> 
> Restore original configuration fix the problem and makes the driver work
> again.
> 
> Also restore the wrongly dropped cpu_to_le32 to correctly support BE
> systems.
> 
> Cc: stable@vger.kernel.org
> Fixes: 0c08080fd71c ("mtd: rawnand: qcom: use FIELD_PREP and GENMASK")
> Tested-by: Robert Marko <robimarko@gmail.com> # IPQ8074 and IPQ6018
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks for the fix!

- Mani

> ---
> Changes v2:
> - Fix smatch warning (add missing cpu_to_le32 that was also dropped
>   from the FIELD_PREP patch)
> 
>  drivers/mtd/nand/raw/qcom_nandc.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index d2d2aeee42a7..6720b547892b 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -1881,18 +1881,18 @@ static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_
>  	nandc->regs->addr0 = 0;
>  	nandc->regs->addr1 = 0;
>  
> -	host->cfg0 = FIELD_PREP(CW_PER_PAGE_MASK, 0) |
> -		     FIELD_PREP(UD_SIZE_BYTES_MASK, 512) |
> -		     FIELD_PREP(NUM_ADDR_CYCLES_MASK, 5) |
> -		     FIELD_PREP(SPARE_SIZE_BYTES_MASK, 0);
> -
> -	host->cfg1 = FIELD_PREP(NAND_RECOVERY_CYCLES_MASK, 7) |
> -		     FIELD_PREP(BAD_BLOCK_BYTE_NUM_MASK, 17) |
> -		     FIELD_PREP(CS_ACTIVE_BSY, 0) |
> -		     FIELD_PREP(BAD_BLOCK_IN_SPARE_AREA, 1) |
> -		     FIELD_PREP(WR_RD_BSY_GAP_MASK, 2) |
> -		     FIELD_PREP(WIDE_FLASH, 0) |
> -		     FIELD_PREP(DEV0_CFG1_ECC_DISABLE, 1);
> +	nandc->regs->cfg0 = cpu_to_le32(FIELD_PREP(CW_PER_PAGE_MASK, 0) |
> +					FIELD_PREP(UD_SIZE_BYTES_MASK, 512) |
> +					FIELD_PREP(NUM_ADDR_CYCLES_MASK, 5) |
> +					FIELD_PREP(SPARE_SIZE_BYTES_MASK, 0));
> +
> +	nandc->regs->cfg1 = cpu_to_le32(FIELD_PREP(NAND_RECOVERY_CYCLES_MASK, 7) |
> +					FIELD_PREP(BAD_BLOCK_BYTE_NUM_MASK, 17) |
> +					FIELD_PREP(CS_ACTIVE_BSY, 0) |
> +					FIELD_PREP(BAD_BLOCK_IN_SPARE_AREA, 1) |
> +					FIELD_PREP(WR_RD_BSY_GAP_MASK, 2) |
> +					FIELD_PREP(WIDE_FLASH, 0) |
> +					FIELD_PREP(DEV0_CFG1_ECC_DISABLE, 1));
>  
>  	if (!nandc->props->qpic_version2)
>  		nandc->regs->ecc_buf_cfg = cpu_to_le32(ECC_CFG_ECC_DISABLE);
> -- 
> 2.47.1
> 

-- 
மணிவண்ணன் சதாசிவம்

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

Re: [PATCH v2] mtd: rawnand: qcom: fix broken config in qcom_param_page_type_exec

[Christian Marangi]

On Mon, Feb 10, 2025 at 10:39:50PM +0530, Manivannan Sadhasivam wrote:
> On Sun, Feb 09, 2025 at 03:09:38PM +0100, Christian Marangi wrote:
> > Fix broken config in qcom_param_page_type_exec caused by copy-paste error
> > from commit 0c08080fd71c ("mtd: rawnand: qcom: use FIELD_PREP and GENMASK")
> > 
> > In qcom_param_page_type_exec the value needs to be set to
> > nandc->regs->cfg0 instead of host->cfg0. This wrong configuration caused
> > the Qcom NANDC driver to malfunction on any device that makes use of it
> > (IPQ806x, IPQ40xx, IPQ807x, IPQ60xx) with the following error:
> > 
> 
> I'm wondering whether the offending commit was really tested or not :(
>

Wellllllll..... It was but it wasn't. The series where this was
introduced was to push the new Qcom QPIC driver driven by a dedicated
SPI controller. That part works and was probably where this was tested.
What was not tested is if these changes affected SNAND driver for other
devices. Saddly it's always difficult to test these changes that affects
lors of devices, in OpenWrt we are working on implementing some kind of testbed
to test images on real devices. (it's currently only an idea and we are
far from having it but it's something I would love to have it so we
could catch these kind of regression faster than getting Issue spammed
with devices getting bricked)

> > [    0.885369] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xaa
> > [    0.885909] nand: Micron NAND 256MiB 1,8V 8-bit
> > [    0.892499] nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
> > [    0.896823] nand: ECC (step, strength) = (512, 8) does not fit in OOB
> > [    0.896836] qcom-nandc 79b0000.nand-controller: No valid ECC settings possible
> > [    0.910996] bam-dma-engine 7984000.dma-controller: Cannot free busy channel
> > [    0.918070] qcom-nandc: probe of 79b0000.nand-controller failed with error -28
> > 
> > Restore original configuration fix the problem and makes the driver work
> > again.
> > 
> > Also restore the wrongly dropped cpu_to_le32 to correctly support BE
> > systems.
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: 0c08080fd71c ("mtd: rawnand: qcom: use FIELD_PREP and GENMASK")
> > Tested-by: Robert Marko <robimarko@gmail.com> # IPQ8074 and IPQ6018
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> Thanks for the fix!
> 
> - Mani
> 
> > ---
> > Changes v2:
> > - Fix smatch warning (add missing cpu_to_le32 that was also dropped
> >   from the FIELD_PREP patch)
> > 
> >  drivers/mtd/nand/raw/qcom_nandc.c | 24 ++++++++++++------------
> >  1 file changed, 12 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> > index d2d2aeee42a7..6720b547892b 100644
> > --- a/drivers/mtd/nand/raw/qcom_nandc.c
> > +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> > @@ -1881,18 +1881,18 @@ static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_
> >  	nandc->regs->addr0 = 0;
> >  	nandc->regs->addr1 = 0;
> >  
> > -	host->cfg0 = FIELD_PREP(CW_PER_PAGE_MASK, 0) |
> > -		     FIELD_PREP(UD_SIZE_BYTES_MASK, 512) |
> > -		     FIELD_PREP(NUM_ADDR_CYCLES_MASK, 5) |
> > -		     FIELD_PREP(SPARE_SIZE_BYTES_MASK, 0);
> > -
> > -	host->cfg1 = FIELD_PREP(NAND_RECOVERY_CYCLES_MASK, 7) |
> > -		     FIELD_PREP(BAD_BLOCK_BYTE_NUM_MASK, 17) |
> > -		     FIELD_PREP(CS_ACTIVE_BSY, 0) |
> > -		     FIELD_PREP(BAD_BLOCK_IN_SPARE_AREA, 1) |
> > -		     FIELD_PREP(WR_RD_BSY_GAP_MASK, 2) |
> > -		     FIELD_PREP(WIDE_FLASH, 0) |
> > -		     FIELD_PREP(DEV0_CFG1_ECC_DISABLE, 1);
> > +	nandc->regs->cfg0 = cpu_to_le32(FIELD_PREP(CW_PER_PAGE_MASK, 0) |
> > +					FIELD_PREP(UD_SIZE_BYTES_MASK, 512) |
> > +					FIELD_PREP(NUM_ADDR_CYCLES_MASK, 5) |
> > +					FIELD_PREP(SPARE_SIZE_BYTES_MASK, 0));
> > +
> > +	nandc->regs->cfg1 = cpu_to_le32(FIELD_PREP(NAND_RECOVERY_CYCLES_MASK, 7) |
> > +					FIELD_PREP(BAD_BLOCK_BYTE_NUM_MASK, 17) |
> > +					FIELD_PREP(CS_ACTIVE_BSY, 0) |
> > +					FIELD_PREP(BAD_BLOCK_IN_SPARE_AREA, 1) |
> > +					FIELD_PREP(WR_RD_BSY_GAP_MASK, 2) |
> > +					FIELD_PREP(WIDE_FLASH, 0) |
> > +					FIELD_PREP(DEV0_CFG1_ECC_DISABLE, 1));
> >  
> >  	if (!nandc->props->qpic_version2)
> >  		nandc->regs->ecc_buf_cfg = cpu_to_le32(ECC_CFG_ECC_DISABLE);
> > -- 
> > 2.47.1
> > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்

-- 
	Ansuel

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

Re: [PATCH v2] mtd: rawnand: qcom: fix broken config in qcom_param_page_type_exec

[Miquel Raynal]

On Sun, 09 Feb 2025 15:09:38 +0100, Christian Marangi wrote:
> Fix broken config in qcom_param_page_type_exec caused by copy-paste error
> from commit 0c08080fd71c ("mtd: rawnand: qcom: use FIELD_PREP and GENMASK")
> 
> In qcom_param_page_type_exec the value needs to be set to
> nandc->regs->cfg0 instead of host->cfg0. This wrong configuration caused
> the Qcom NANDC driver to malfunction on any device that makes use of it
> (IPQ806x, IPQ40xx, IPQ807x, IPQ60xx) with the following error:
> 
> [...]

Applied to mtd/fixes, thanks!

[1/1] mtd: rawnand: qcom: fix broken config in qcom_param_page_type_exec
      commit: 86ede0a61f8576a84bb0a93c5d9861d2ec1cdf9a

Patche(s) should be available on mtd/linux.git and will be
part of the next PR (provided that no robot complains by then).

Kind regards,
Miquèl


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