Re: [PATCH 4/7] spi: spi-qpic-snand: use the common qcom_nandc_config_cw_write() function - sashiko-bot

From: sashiko-bot@kernel.org
To: "Gabor Juhos" <j4g8y7@gmail.com>, "Mark Brown" <broonie@kernel.org>
Cc: "Md Sadre Alam" <md.alam@oss.qualcomm.com>,
	"Sricharan Ramabadhran" <quic_srichara@quicinc.com>,
	"Varadarajan Narayanan" <varadarajan.narayanan@oss.qualcomm.com>,
	linux-mtd@lists.infradead.org
Subject: Re: [PATCH 4/7] spi: spi-qpic-snand: use the common qcom_nandc_config_cw_write() function
Date: Mon, 09 Nov 2026 19:45:48 +0000	[thread overview]
Message-ID: <sashiko-outbox-205501@kernel.org> (raw)
In-Reply-To: <20261109-qpic-snand-use-common-functions-v1-4-1e8601e2db84@gmail.com>

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] spi: qpic-snand: improper raw OOB write handling in
  qcom_spi_program_execute() [1]
--

commit 31ce15b93da7dda82ea19398b4abcc5e1c175880
Author: Gabor Juhos <j4g8y7@gmail.com>

spi: spi-qpic-snand: use the common qcom_nandc_config_cw_write() function

This commit replaces the local qcom_spi_config_cw_write() helper with the
common qcom_nandc_config_cw_write() function from qpic_common.

> diff --git a/drivers/spi/spi-qpic-snand.c b/drivers/spi/spi-qpic-snand.c
> index a825bdb93a8a1..6774a0a5db2a7 100644
> --- a/drivers/spi/spi-qpic-snand.c
> +++ b/drivers/spi/spi-qpic-snand.c

[ ... ]

> @@ -1271,7 +1260,7 @@ static int qcom_spi_program_oob(struct qcom_nand_controller *snandc,
>  				    oob_buf, 0, snandc->qspi->mtd->oobavail);
>  	qcom_nandc_config_page_write(snandc);
>  	qcom_write_data_dma(snandc, FLASH_BUF_ACC, snandc->data_buffer, data_size + oob_size, 0);
> -	qcom_spi_config_cw_write(snandc);
> +	qcom_nandc_config_cw_write(snandc);
>  
>  	ret = qcom_submit_descs(snandc);
>  	if (ret) {

[Severity: Critical]
This problem wasn't introduced by this patch, but does qcom_spi_program_oob()
[1] silently discard raw OOB writes such as bad block markers from
spinand_markbad()?

spinand_markbad() issues a raw OOB write with datalen = 0, ooblen = 2, and
mode = MTD_OPS_RAW, which sets page_rw = false, oob_rw = true, and
raw_rw = true in qcom_spi_ecc_prepare_io_req_pipelined().  However,
qcom_spi_program_execute() ignores raw_rw when page_rw is false and routes
the write to qcom_spi_program_oob():

drivers/spi/spi-qpic-snand.c:qcom_spi_program_execute() {
	if (snandc->qspi->page_rw && snandc->qspi->raw_rw)
		return qcom_spi_program_raw(snandc, op);

	if (snandc->qspi->page_rw)
		return qcom_spi_program_ecc(snandc, op);

	if (snandc->qspi->oob_rw)
		return qcom_spi_program_oob(snandc, op);
    ...
}

qcom_spi_program_oob() sets oob_buf to snandc->qspi->data_buf (the start of
the page buffer rather than the OOB offset) and unconditionally enables ECC
via ecc_cfg->ecc_bch_cfg.  As a result, mtd_ooblayout_get_databytes() reads
0xff instead of the bad block marker and programs it to flash with ECC
enabled, leaving defective blocks unmarked and allowing subsequent writes to
failing blocks.

[1] https://sashiko.dev/#/bug/linux-da37c243-e4f1-4870-8190-46ae2f5b3282

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-qpic-snand-use-common-functions-v1-0-1e8601e2db84@gmail.com?part=4

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