linux-kernel - Re: [PATCH v2] mtd: rawnand: qcom: fix broken config in qcom_param_page_type

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <67aa667e.5d0a0220.10ff3e.24a6@mx.google.com>
Date: Mon, 10 Feb 2025 21:50:03 +0100
From: Christian Marangi <ansuelsmth@...il.com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Cc: Miquel Raynal <miquel.raynal@...tlin.com>,
	Richard Weinberger <richard@....at>,
	Vignesh Raghavendra <vigneshr@...com>,
	Md Sadre Alam <quic_mdalam@...cinc.com>,
	Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
	linux-mtd@...ts.infradead.org, linux-arm-msm@...r.kernel.org,
	linux-kernel@...r.kernel.org, stable@...r.kernel.org,
	Robert Marko <robimarko@...il.com>
Subject: Re: [PATCH v2] mtd: rawnand: qcom: fix broken config in
 qcom_param_page_type_exec

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@...r.kernel.org
> > Fixes: 0c08080fd71c ("mtd: rawnand: qcom: use FIELD_PREP and GENMASK")
> > Tested-by: Robert Marko <robimarko@...il.com> # IPQ8074 and IPQ6018
> > Signed-off-by: Christian Marangi <ansuelsmth@...il.com>
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.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