During raw read, neither the status of the ECC correction nor the erased
state of the codeword gets checked by the qcom_spi_read_cw_raw() function,
so in case of raw access reading the corresponding registers via DMA is
superfluous.

Extend the qcom_spi_config_cw_read() function to evaluate the existing
(but actually unused) 'use_ecc' parameter, and configure reading only
the flash status register when ECC is not used.

With the change, the code gets in line with the corresponding part of
the config_nand_cw_read() function in the qcom_nandc driver.

Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
---
 drivers/spi/spi-qpic-snand.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-qpic-snand.c b/drivers/spi/spi-qpic-snand.c
index 7b76d2c82a5287df13ee6fcebc4abbe58ca861ee..119003c4784890458a41c67fa8bc17d721030b0d 100644
--- a/drivers/spi/spi-qpic-snand.c
+++ b/drivers/spi/spi-qpic-snand.c
@@ -494,9 +494,14 @@ qcom_spi_config_cw_read(struct qcom_nand_controller *snandc, bool use_ecc, int c
 	qcom_write_reg_dma(snandc, &snandc->regs->cmd, NAND_FLASH_CMD, 1, NAND_BAM_NEXT_SGL);
 	qcom_write_reg_dma(snandc, &snandc->regs->exec, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
 
-	qcom_read_reg_dma(snandc, NAND_FLASH_STATUS, 2, 0);
-	qcom_read_reg_dma(snandc, NAND_ERASED_CW_DETECT_STATUS, 1,
-			  NAND_BAM_NEXT_SGL);
+	if (use_ecc) {
+		qcom_read_reg_dma(snandc, NAND_FLASH_STATUS, 2, 0);
+		qcom_read_reg_dma(snandc, NAND_ERASED_CW_DETECT_STATUS, 1,
+				  NAND_BAM_NEXT_SGL);
+	} else {
+		qcom_read_reg_dma(snandc, NAND_FLASH_STATUS, 1,
+				  NAND_BAM_NEXT_SGL);
+	}
 }
 
 static int qcom_spi_block_erase(struct qcom_nand_controller *snandc)

---
base-commit: 13d0fe84a214658254a7412b2b46ec1507dc51f0
change-id: 20250805-qpic-snand-handle-use_ecc-de929376d50b

Best regards,
-- 
Gabor Juhos <j4g8y7@gmail.com>

On Fri, 08 Aug 2025 19:15:01 +0200, Gabor Juhos wrote:
> During raw read, neither the status of the ECC correction nor the erased
> state of the codeword gets checked by the qcom_spi_read_cw_raw() function,
> so in case of raw access reading the corresponding registers via DMA is
> superfluous.
> 
> Extend the qcom_spi_config_cw_read() function to evaluate the existing
> (but actually unused) 'use_ecc' parameter, and configure reading only
> the flash status register when ECC is not used.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: spi-qpic-snand: handle 'use_ecc' parameter of qcom_spi_config_cw_read()
      commit: 9c45f95222beecd6a284fd1284d54dd7a772cf59

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

On 8/8/25 7:15 PM, Gabor Juhos wrote:
> During raw read, neither the status of the ECC correction nor the erased
> state of the codeword gets checked by the qcom_spi_read_cw_raw() function,
> so in case of raw access reading the corresponding registers via DMA is
> superfluous.
> 
> Extend the qcom_spi_config_cw_read() function to evaluate the existing
> (but actually unused) 'use_ecc' parameter, and configure reading only
> the flash status register when ECC is not used.
> 
> With the change, the code gets in line with the corresponding part of
> the config_nand_cw_read() function in the qcom_nandc driver.
> 
> Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
> ---
>  drivers/spi/spi-qpic-snand.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/spi/spi-qpic-snand.c b/drivers/spi/spi-qpic-snand.c
> index 7b76d2c82a5287df13ee6fcebc4abbe58ca861ee..119003c4784890458a41c67fa8bc17d721030b0d 100644
> --- a/drivers/spi/spi-qpic-snand.c
> +++ b/drivers/spi/spi-qpic-snand.c
> @@ -494,9 +494,14 @@ qcom_spi_config_cw_read(struct qcom_nand_controller *snandc, bool use_ecc, int c
>  	qcom_write_reg_dma(snandc, &snandc->regs->cmd, NAND_FLASH_CMD, 1, NAND_BAM_NEXT_SGL);
>  	qcom_write_reg_dma(snandc, &snandc->regs->exec, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
>  
> -	qcom_read_reg_dma(snandc, NAND_FLASH_STATUS, 2, 0);
> -	qcom_read_reg_dma(snandc, NAND_ERASED_CW_DETECT_STATUS, 1,
> -			  NAND_BAM_NEXT_SGL);
> +	if (use_ecc) {
> +		qcom_read_reg_dma(snandc, NAND_FLASH_STATUS, 2, 0);

Why are we reading 2 registers (the 2 in the func call) here, but 1 everywhere
else?

Konrad

> +		qcom_read_reg_dma(snandc, NAND_ERASED_CW_DETECT_STATUS, 1,
> +				  NAND_BAM_NEXT_SGL);
> +	} else {
> +		qcom_read_reg_dma(snandc, NAND_FLASH_STATUS, 1,
> +				  NAND_BAM_NEXT_SGL);
> +	}
>  }
>  
>  static int qcom_spi_block_erase(struct qcom_nand_controller *snandc)
> 
> ---
> base-commit: 13d0fe84a214658254a7412b2b46ec1507dc51f0
> change-id: 20250805-qpic-snand-handle-use_ecc-de929376d50b
> 
> Best regards,

2025. 08. 12. 11:55 keltezéssel, Konrad Dybcio írta:
> On 8/8/25 7:15 PM, Gabor Juhos wrote:
>> During raw read, neither the status of the ECC correction nor the erased
>> state of the codeword gets checked by the qcom_spi_read_cw_raw() function,
>> so in case of raw access reading the corresponding registers via DMA is
>> superfluous.
>>
>> Extend the qcom_spi_config_cw_read() function to evaluate the existing
>> (but actually unused) 'use_ecc' parameter, and configure reading only
>> the flash status register when ECC is not used.
>>
>> With the change, the code gets in line with the corresponding part of
>> the config_nand_cw_read() function in the qcom_nandc driver.
>>
>> Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
>> ---
>>  drivers/spi/spi-qpic-snand.c | 11 ++++++++---
>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/spi/spi-qpic-snand.c b/drivers/spi/spi-qpic-snand.c
>> index 7b76d2c82a5287df13ee6fcebc4abbe58ca861ee..119003c4784890458a41c67fa8bc17d721030b0d 100644
>> --- a/drivers/spi/spi-qpic-snand.c
>> +++ b/drivers/spi/spi-qpic-snand.c
>> @@ -494,9 +494,14 @@ qcom_spi_config_cw_read(struct qcom_nand_controller *snandc, bool use_ecc, int c
>>  	qcom_write_reg_dma(snandc, &snandc->regs->cmd, NAND_FLASH_CMD, 1, NAND_BAM_NEXT_SGL);
>>  	qcom_write_reg_dma(snandc, &snandc->regs->exec, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
>>  
>> -	qcom_read_reg_dma(snandc, NAND_FLASH_STATUS, 2, 0);
>> -	qcom_read_reg_dma(snandc, NAND_ERASED_CW_DETECT_STATUS, 1,
>> -			  NAND_BAM_NEXT_SGL);
>> +	if (use_ecc) {
>> +		qcom_read_reg_dma(snandc, NAND_FLASH_STATUS, 2, 0);
> 
> Why are we reading 2 registers (the 2 in the func call) here, ...

Because when ECC is used, we need the status of the ECC correction from the
NAND_BUFFER_STATUS register which is placed right after the NAND_FLASH_STATUS.

Here are the relevant definitions from the 'nand-qpic-common.h' header for
reference:

#define	NAND_FLASH_STATUS		0x14
#define	NAND_BUFFER_STATUS		0x18

So the two registers can be read with a single DMA operation.

> ... but 1 everywhere else?

When ECC is not used, we only need the value from the NAND_FLASH_STATUS
register, so we don't have to read two registers.

Regards,
Gabor

On 8/12/25 1:06 PM, Gabor Juhos wrote:
> 2025. 08. 12. 11:55 keltezéssel, Konrad Dybcio írta:
>> On 8/8/25 7:15 PM, Gabor Juhos wrote:
>>> During raw read, neither the status of the ECC correction nor the erased
>>> state of the codeword gets checked by the qcom_spi_read_cw_raw() function,
>>> so in case of raw access reading the corresponding registers via DMA is
>>> superfluous.
>>>
>>> Extend the qcom_spi_config_cw_read() function to evaluate the existing
>>> (but actually unused) 'use_ecc' parameter, and configure reading only
>>> the flash status register when ECC is not used.
>>>
>>> With the change, the code gets in line with the corresponding part of
>>> the config_nand_cw_read() function in the qcom_nandc driver.
>>>
>>> Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
>>> ---
>>>  drivers/spi/spi-qpic-snand.c | 11 ++++++++---
>>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/spi/spi-qpic-snand.c b/drivers/spi/spi-qpic-snand.c
>>> index 7b76d2c82a5287df13ee6fcebc4abbe58ca861ee..119003c4784890458a41c67fa8bc17d721030b0d 100644
>>> --- a/drivers/spi/spi-qpic-snand.c
>>> +++ b/drivers/spi/spi-qpic-snand.c
>>> @@ -494,9 +494,14 @@ qcom_spi_config_cw_read(struct qcom_nand_controller *snandc, bool use_ecc, int c
>>>  	qcom_write_reg_dma(snandc, &snandc->regs->cmd, NAND_FLASH_CMD, 1, NAND_BAM_NEXT_SGL);
>>>  	qcom_write_reg_dma(snandc, &snandc->regs->exec, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
>>>  
>>> -	qcom_read_reg_dma(snandc, NAND_FLASH_STATUS, 2, 0);
>>> -	qcom_read_reg_dma(snandc, NAND_ERASED_CW_DETECT_STATUS, 1,
>>> -			  NAND_BAM_NEXT_SGL);
>>> +	if (use_ecc) {
>>> +		qcom_read_reg_dma(snandc, NAND_FLASH_STATUS, 2, 0);
>>
>> Why are we reading 2 registers (the 2 in the func call) here, ...
> 
> Because when ECC is used, we need the status of the ECC correction from the
> NAND_BUFFER_STATUS register which is placed right after the NAND_FLASH_STATUS.
> 
> Here are the relevant definitions from the 'nand-qpic-common.h' header for
> reference:
> 
> #define	NAND_FLASH_STATUS		0x14
> #define	NAND_BUFFER_STATUS		0x18
> 
> So the two registers can be read with a single DMA operation.
> 
>> ... but 1 everywhere else?
> 
> When ECC is not used, we only need the value from the NAND_FLASH_STATUS
> register, so we don't have to read two registers.

OK yeah I can see that

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad