[PATCH v4 0/3] QPIC v2 fixes for SDX75

[PATCH v4 0/3] QPIC v2 fixes for SDX75

[Md Sadre Alam]

The BAM command descriptor provides only 18 bits to specify the NAND
register offset. Additionally, in the BAM command descriptor, the NAND
register offset is supposed to be specified as "(NANDc base - BAM base)
+ reg_off". Since, the BAM controller expecting the value in the form of
"NANDc base - BAM base", so that added a new field 'bam_offset' in the
NAND properties structure and use it while preparing the command descriptor.

Previously, the driver was specifying the NANDc base address in the BAM
command descriptor.

also fixing last codeword read in qcom_param_page_type_exec() and read
len for onfi param page.

v4:
 * updated commit message
 * Renamed nand_offset to bam_offset
 * Dropped this patch "spi: spi-qpic-snand: set nand_offset for ipq9574"
   as "Gabor" suggested at [1]
   [1]:https://lore.kernel.org/lkml/a72f0db0-fd49-4cff-bffe-671fc757a219@gmail.com/
 * Removed nandc_reg_phys(chip, offset) macro and implemented this in function
   itself
 * Merged "spi: spi-qpic-snand: set bam_offset for ipq9574" in 
   "mtd: rawnand: qcom: Pass 18 bit offset from QPIC base address to BAM"

v3:
 * Updated commit message
 * updated qpic_v2 to qpic_version2
 * Removed dev_cmd_reg_start = 0 in sdx55_nandc_props {}
 * Added new patch to add nand_offset in spi_qpic_snand
   driver

v2:
 * Updated commit message
 * Added stable kernel tag
 * Added Fixes tag
 * Renamed the variable from offset_from_qpic to nandc_offset
 * Set buf_count to 512 in the parameter page read
 * Replaced the buf_count value of 512 with the len in bytes 

v1:
 * These patches will fix the following:
 * 1) onfi param page read which was broken by exec_op() patch.
 * 2) Fixed offset passed to BAM from QPIC base


Md Sadre Alam (3):
  mtd: rawnand: qcom: Pass 18 bit offset from NANDc base to BAM base
  mtd: rawnand: qcom: Fix last codeword read in
    qcom_param_page_type_exec()
  mtd: rawnand: qcom: Fix read len for onfi param page

 drivers/mtd/nand/qpic_common.c       |  8 ++++----
 drivers/mtd/nand/raw/qcom_nandc.c    | 18 +++++++++++++++---
 drivers/spi/spi-qpic-snand.c         |  1 +
 include/linux/mtd/nand-qpic-common.h |  4 +---
 4 files changed, 21 insertions(+), 10 deletions(-)

-- 
2.34.1

[PATCH v4 1/3] mtd: rawnand: qcom: Pass 18 bit offset from NANDc base to BAM base

[Md Sadre Alam]

The BAM command descriptor provides only 18 bits to specify the NAND
register offset. Additionally, in the BAM command descriptor, the NAND
register offset is supposed to be specified as "(NANDc base - BAM base)
+ reg_off". Since, the BAM controller expecting the value in the form of
"NANDc base - BAM base", so that added a new field 'bam_offset' in the NAND
properties structure and use it while preparing the command descriptor.

Previously, the driver was specifying the NANDc base address in the BAM
command descriptor.

Cc: stable@vger.kernel.org
Fixes: 8d6b6d7e135e ("mtd: nand: qcom: support for command descriptor formation")
Tested-by: Lakshmi Sowjanya D <quic_laksd@quicinc.com>
Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---
 drivers/mtd/nand/qpic_common.c       | 8 ++++----
 drivers/mtd/nand/raw/qcom_nandc.c    | 4 ++++
 drivers/spi/spi-qpic-snand.c         | 1 +
 include/linux/mtd/nand-qpic-common.h | 4 +---
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/qpic_common.c b/drivers/mtd/nand/qpic_common.c
index e0ed25b5afea..4dc4d65e7d32 100644
--- a/drivers/mtd/nand/qpic_common.c
+++ b/drivers/mtd/nand/qpic_common.c
@@ -236,21 +236,21 @@ int qcom_prep_bam_dma_desc_cmd(struct qcom_nand_controller *nandc, bool read,
 	int i, ret;
 	struct bam_cmd_element *bam_ce_buffer;
 	struct bam_transaction *bam_txn = nandc->bam_txn;
+	u32 offset;
 
 	bam_ce_buffer = &bam_txn->bam_ce[bam_txn->bam_ce_pos];
 
 	/* fill the command desc */
 	for (i = 0; i < size; i++) {
+		offset = nandc->props->bam_offset + reg_off + 4 * i;
 		if (read)
 			bam_prep_ce(&bam_ce_buffer[i],
-				    nandc_reg_phys(nandc, reg_off + 4 * i),
-				    BAM_READ_COMMAND,
+				    offset, BAM_READ_COMMAND,
 				    reg_buf_dma_addr(nandc,
 						     (__le32 *)vaddr + i));
 		else
 			bam_prep_ce_le32(&bam_ce_buffer[i],
-					 nandc_reg_phys(nandc, reg_off + 4 * i),
-					 BAM_WRITE_COMMAND,
+					 offset, BAM_WRITE_COMMAND,
 					 *((__le32 *)vaddr + i));
 	}
 
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 5eaa0be367cd..ef2dd158ca34 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -2360,6 +2360,7 @@ static const struct qcom_nandc_props ipq806x_nandc_props = {
 	.supports_bam = false,
 	.use_codeword_fixup = true,
 	.dev_cmd_reg_start = 0x0,
+	.bam_offset = 0x30000,
 };
 
 static const struct qcom_nandc_props ipq4019_nandc_props = {
@@ -2367,6 +2368,7 @@ static const struct qcom_nandc_props ipq4019_nandc_props = {
 	.supports_bam = true,
 	.nandc_part_of_qpic = true,
 	.dev_cmd_reg_start = 0x0,
+	.bam_offset = 0x30000,
 };
 
 static const struct qcom_nandc_props ipq8074_nandc_props = {
@@ -2374,6 +2376,7 @@ static const struct qcom_nandc_props ipq8074_nandc_props = {
 	.supports_bam = true,
 	.nandc_part_of_qpic = true,
 	.dev_cmd_reg_start = 0x7000,
+	.bam_offset = 0x30000,
 };
 
 static const struct qcom_nandc_props sdx55_nandc_props = {
@@ -2382,6 +2385,7 @@ static const struct qcom_nandc_props sdx55_nandc_props = {
 	.nandc_part_of_qpic = true,
 	.qpic_version2 = true,
 	.dev_cmd_reg_start = 0x7000,
+	.bam_offset = 0x30000,
 };
 
 /*
diff --git a/drivers/spi/spi-qpic-snand.c b/drivers/spi/spi-qpic-snand.c
index a2af731049d4..89c774bf85a6 100644
--- a/drivers/spi/spi-qpic-snand.c
+++ b/drivers/spi/spi-qpic-snand.c
@@ -1605,6 +1605,7 @@ static void qcom_spi_remove(struct platform_device *pdev)
 
 static const struct qcom_nandc_props ipq9574_snandc_props = {
 	.dev_cmd_reg_start = 0x7000,
+	.bam_offset = 0x30000,
 	.supports_bam = true,
 };
 
diff --git a/include/linux/mtd/nand-qpic-common.h b/include/linux/mtd/nand-qpic-common.h
index cd7172e6c1bb..e8462deda6db 100644
--- a/include/linux/mtd/nand-qpic-common.h
+++ b/include/linux/mtd/nand-qpic-common.h
@@ -199,9 +199,6 @@
  */
 #define dev_cmd_reg_addr(nandc, reg) ((nandc)->props->dev_cmd_reg_start + (reg))
 
-/* Returns the NAND register physical address */
-#define nandc_reg_phys(chip, offset) ((chip)->base_phys + (offset))
-
 /* Returns the dma address for reg read buffer */
 #define reg_buf_dma_addr(chip, vaddr) \
 	((chip)->reg_read_dma + \
@@ -454,6 +451,7 @@ struct qcom_nand_controller {
 struct qcom_nandc_props {
 	u32 ecc_modes;
 	u32 dev_cmd_reg_start;
+	u32 bam_offset;
 	bool supports_bam;
 	bool nandc_part_of_qpic;
 	bool qpic_version2;
-- 
2.34.1

Re: [PATCH v4 1/3] mtd: rawnand: qcom: Pass 18 bit offset from NANDc base to BAM base

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

On Thu, Apr 10, 2025 at 03:30:17PM +0530, Md Sadre Alam wrote:
> The BAM command descriptor provides only 18 bits to specify the NAND
> register offset. Additionally, in the BAM command descriptor, the NAND
> register offset is supposed to be specified as "(NANDc base - BAM base)
> + reg_off". Since, the BAM controller expecting the value in the form of
> "NANDc base - BAM base", so that added a new field 'bam_offset' in the NAND
> properties structure and use it while preparing the command descriptor.

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 1/3] mtd: rawnand: qcom: Pass 18 bit offset from NANDc base to BAM base

[Gabor Juhos]

2025. 04. 10. 12:00 keltezéssel, Md Sadre Alam írta:
> The BAM command descriptor provides only 18 bits to specify the NAND
> register offset. Additionally, in the BAM command descriptor, the NAND
> register offset is supposed to be specified as "(NANDc base - BAM base)
> + reg_off". Since, the BAM controller expecting the value in the form of
> "NANDc base - BAM base", so that added a new field 'bam_offset' in the NAND
> properties structure and use it while preparing the command descriptor.
> 
> Previously, the driver was specifying the NANDc base address in the BAM
> command descriptor.
> 
> Cc: stable@vger.kernel.org
> Fixes: 8d6b6d7e135e ("mtd: nand: qcom: support for command descriptor formation")
> Tested-by: Lakshmi Sowjanya D <quic_laksd@quicinc.com>
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>

Tested-by: Gabor Juhos <j4g8y7@gmail.com> # on IPQ9574

Re: [PATCH v4 1/3] mtd: rawnand: qcom: Pass 18 bit offset from NANDc base to BAM base

[Manivannan Sadhasivam]

On Thu, Apr 10, 2025 at 03:30:17PM +0530, Md Sadre Alam wrote:
> The BAM command descriptor provides only 18 bits to specify the NAND
> register offset. 

s/NAND register offset/BAM register offset

> Additionally, in the BAM command descriptor, the NAND
> register offset is supposed to be specified as "(NANDc base - BAM base)

Same here.

> + reg_off". Since, the BAM controller expecting the value in the form of
> "NANDc base - BAM base", so that added a new field 'bam_offset' in the NAND
> properties structure and use it while preparing the command descriptor.
> 
> Previously, the driver was specifying the NANDc base address in the BAM
> command descriptor.
> 
> Cc: stable@vger.kernel.org
> Fixes: 8d6b6d7e135e ("mtd: nand: qcom: support for command descriptor formation")
> Tested-by: Lakshmi Sowjanya D <quic_laksd@quicinc.com>
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>

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

- Mani

> ---
>  drivers/mtd/nand/qpic_common.c       | 8 ++++----
>  drivers/mtd/nand/raw/qcom_nandc.c    | 4 ++++
>  drivers/spi/spi-qpic-snand.c         | 1 +
>  include/linux/mtd/nand-qpic-common.h | 4 +---
>  4 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mtd/nand/qpic_common.c b/drivers/mtd/nand/qpic_common.c
> index e0ed25b5afea..4dc4d65e7d32 100644
> --- a/drivers/mtd/nand/qpic_common.c
> +++ b/drivers/mtd/nand/qpic_common.c
> @@ -236,21 +236,21 @@ int qcom_prep_bam_dma_desc_cmd(struct qcom_nand_controller *nandc, bool read,
>  	int i, ret;
>  	struct bam_cmd_element *bam_ce_buffer;
>  	struct bam_transaction *bam_txn = nandc->bam_txn;
> +	u32 offset;
>  
>  	bam_ce_buffer = &bam_txn->bam_ce[bam_txn->bam_ce_pos];
>  
>  	/* fill the command desc */
>  	for (i = 0; i < size; i++) {
> +		offset = nandc->props->bam_offset + reg_off + 4 * i;
>  		if (read)
>  			bam_prep_ce(&bam_ce_buffer[i],
> -				    nandc_reg_phys(nandc, reg_off + 4 * i),
> -				    BAM_READ_COMMAND,
> +				    offset, BAM_READ_COMMAND,
>  				    reg_buf_dma_addr(nandc,
>  						     (__le32 *)vaddr + i));
>  		else
>  			bam_prep_ce_le32(&bam_ce_buffer[i],
> -					 nandc_reg_phys(nandc, reg_off + 4 * i),
> -					 BAM_WRITE_COMMAND,
> +					 offset, BAM_WRITE_COMMAND,
>  					 *((__le32 *)vaddr + i));
>  	}
>  
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index 5eaa0be367cd..ef2dd158ca34 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -2360,6 +2360,7 @@ static const struct qcom_nandc_props ipq806x_nandc_props = {
>  	.supports_bam = false,
>  	.use_codeword_fixup = true,
>  	.dev_cmd_reg_start = 0x0,
> +	.bam_offset = 0x30000,
>  };
>  
>  static const struct qcom_nandc_props ipq4019_nandc_props = {
> @@ -2367,6 +2368,7 @@ static const struct qcom_nandc_props ipq4019_nandc_props = {
>  	.supports_bam = true,
>  	.nandc_part_of_qpic = true,
>  	.dev_cmd_reg_start = 0x0,
> +	.bam_offset = 0x30000,
>  };
>  
>  static const struct qcom_nandc_props ipq8074_nandc_props = {
> @@ -2374,6 +2376,7 @@ static const struct qcom_nandc_props ipq8074_nandc_props = {
>  	.supports_bam = true,
>  	.nandc_part_of_qpic = true,
>  	.dev_cmd_reg_start = 0x7000,
> +	.bam_offset = 0x30000,
>  };
>  
>  static const struct qcom_nandc_props sdx55_nandc_props = {
> @@ -2382,6 +2385,7 @@ static const struct qcom_nandc_props sdx55_nandc_props = {
>  	.nandc_part_of_qpic = true,
>  	.qpic_version2 = true,
>  	.dev_cmd_reg_start = 0x7000,
> +	.bam_offset = 0x30000,
>  };
>  
>  /*
> diff --git a/drivers/spi/spi-qpic-snand.c b/drivers/spi/spi-qpic-snand.c
> index a2af731049d4..89c774bf85a6 100644
> --- a/drivers/spi/spi-qpic-snand.c
> +++ b/drivers/spi/spi-qpic-snand.c
> @@ -1605,6 +1605,7 @@ static void qcom_spi_remove(struct platform_device *pdev)
>  
>  static const struct qcom_nandc_props ipq9574_snandc_props = {
>  	.dev_cmd_reg_start = 0x7000,
> +	.bam_offset = 0x30000,
>  	.supports_bam = true,
>  };
>  
> diff --git a/include/linux/mtd/nand-qpic-common.h b/include/linux/mtd/nand-qpic-common.h
> index cd7172e6c1bb..e8462deda6db 100644
> --- a/include/linux/mtd/nand-qpic-common.h
> +++ b/include/linux/mtd/nand-qpic-common.h
> @@ -199,9 +199,6 @@
>   */
>  #define dev_cmd_reg_addr(nandc, reg) ((nandc)->props->dev_cmd_reg_start + (reg))
>  
> -/* Returns the NAND register physical address */
> -#define nandc_reg_phys(chip, offset) ((chip)->base_phys + (offset))
> -
>  /* Returns the dma address for reg read buffer */
>  #define reg_buf_dma_addr(chip, vaddr) \
>  	((chip)->reg_read_dma + \
> @@ -454,6 +451,7 @@ struct qcom_nand_controller {
>  struct qcom_nandc_props {
>  	u32 ecc_modes;
>  	u32 dev_cmd_reg_start;
> +	u32 bam_offset;
>  	bool supports_bam;
>  	bool nandc_part_of_qpic;
>  	bool qpic_version2;
> -- 
> 2.34.1
> 

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

Re: [PATCH v4 1/3] mtd: rawnand: qcom: Pass 18 bit offset from NANDc base to BAM base

[Md Sadre Alam]

On 4/25/2025 1:17 PM, Manivannan Sadhasivam wrote:
> On Thu, Apr 10, 2025 at 03:30:17PM +0530, Md Sadre Alam wrote:
>> The BAM command descriptor provides only 18 bits to specify the NAND
>> register offset.
> 
> s/NAND register offset/BAM register offset
Ok
> 
>> Additionally, in the BAM command descriptor, the NAND
>> register offset is supposed to be specified as "(NANDc base - BAM base)
> 
> Same here.
Ok
> 
>> + reg_off". Since, the BAM controller expecting the value in the form of
>> "NANDc base - BAM base", so that added a new field 'bam_offset' in the NAND
>> properties structure and use it while preparing the command descriptor.
>>
>> Previously, the driver was specifying the NANDc base address in the BAM
>> command descriptor.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 8d6b6d7e135e ("mtd: nand: qcom: support for command descriptor formation")
>> Tested-by: Lakshmi Sowjanya D <quic_laksd@quicinc.com>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> - Mani
> 
>> ---
>>   drivers/mtd/nand/qpic_common.c       | 8 ++++----
>>   drivers/mtd/nand/raw/qcom_nandc.c    | 4 ++++
>>   drivers/spi/spi-qpic-snand.c         | 1 +
>>   include/linux/mtd/nand-qpic-common.h | 4 +---
>>   4 files changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/qpic_common.c b/drivers/mtd/nand/qpic_common.c
>> index e0ed25b5afea..4dc4d65e7d32 100644
>> --- a/drivers/mtd/nand/qpic_common.c
>> +++ b/drivers/mtd/nand/qpic_common.c
>> @@ -236,21 +236,21 @@ int qcom_prep_bam_dma_desc_cmd(struct qcom_nand_controller *nandc, bool read,
>>   	int i, ret;
>>   	struct bam_cmd_element *bam_ce_buffer;
>>   	struct bam_transaction *bam_txn = nandc->bam_txn;
>> +	u32 offset;
>>   
>>   	bam_ce_buffer = &bam_txn->bam_ce[bam_txn->bam_ce_pos];
>>   
>>   	/* fill the command desc */
>>   	for (i = 0; i < size; i++) {
>> +		offset = nandc->props->bam_offset + reg_off + 4 * i;
>>   		if (read)
>>   			bam_prep_ce(&bam_ce_buffer[i],
>> -				    nandc_reg_phys(nandc, reg_off + 4 * i),
>> -				    BAM_READ_COMMAND,
>> +				    offset, BAM_READ_COMMAND,
>>   				    reg_buf_dma_addr(nandc,
>>   						     (__le32 *)vaddr + i));
>>   		else
>>   			bam_prep_ce_le32(&bam_ce_buffer[i],
>> -					 nandc_reg_phys(nandc, reg_off + 4 * i),
>> -					 BAM_WRITE_COMMAND,
>> +					 offset, BAM_WRITE_COMMAND,
>>   					 *((__le32 *)vaddr + i));
>>   	}
>>   
>> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
>> index 5eaa0be367cd..ef2dd158ca34 100644
>> --- a/drivers/mtd/nand/raw/qcom_nandc.c
>> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
>> @@ -2360,6 +2360,7 @@ static const struct qcom_nandc_props ipq806x_nandc_props = {
>>   	.supports_bam = false,
>>   	.use_codeword_fixup = true,
>>   	.dev_cmd_reg_start = 0x0,
>> +	.bam_offset = 0x30000,
>>   };
>>   
>>   static const struct qcom_nandc_props ipq4019_nandc_props = {
>> @@ -2367,6 +2368,7 @@ static const struct qcom_nandc_props ipq4019_nandc_props = {
>>   	.supports_bam = true,
>>   	.nandc_part_of_qpic = true,
>>   	.dev_cmd_reg_start = 0x0,
>> +	.bam_offset = 0x30000,
>>   };
>>   
>>   static const struct qcom_nandc_props ipq8074_nandc_props = {
>> @@ -2374,6 +2376,7 @@ static const struct qcom_nandc_props ipq8074_nandc_props = {
>>   	.supports_bam = true,
>>   	.nandc_part_of_qpic = true,
>>   	.dev_cmd_reg_start = 0x7000,
>> +	.bam_offset = 0x30000,
>>   };
>>   
>>   static const struct qcom_nandc_props sdx55_nandc_props = {
>> @@ -2382,6 +2385,7 @@ static const struct qcom_nandc_props sdx55_nandc_props = {
>>   	.nandc_part_of_qpic = true,
>>   	.qpic_version2 = true,
>>   	.dev_cmd_reg_start = 0x7000,
>> +	.bam_offset = 0x30000,
>>   };
>>   
>>   /*
>> diff --git a/drivers/spi/spi-qpic-snand.c b/drivers/spi/spi-qpic-snand.c
>> index a2af731049d4..89c774bf85a6 100644
>> --- a/drivers/spi/spi-qpic-snand.c
>> +++ b/drivers/spi/spi-qpic-snand.c
>> @@ -1605,6 +1605,7 @@ static void qcom_spi_remove(struct platform_device *pdev)
>>   
>>   static const struct qcom_nandc_props ipq9574_snandc_props = {
>>   	.dev_cmd_reg_start = 0x7000,
>> +	.bam_offset = 0x30000,
>>   	.supports_bam = true,
>>   };
>>   
>> diff --git a/include/linux/mtd/nand-qpic-common.h b/include/linux/mtd/nand-qpic-common.h
>> index cd7172e6c1bb..e8462deda6db 100644
>> --- a/include/linux/mtd/nand-qpic-common.h
>> +++ b/include/linux/mtd/nand-qpic-common.h
>> @@ -199,9 +199,6 @@
>>    */
>>   #define dev_cmd_reg_addr(nandc, reg) ((nandc)->props->dev_cmd_reg_start + (reg))
>>   
>> -/* Returns the NAND register physical address */
>> -#define nandc_reg_phys(chip, offset) ((chip)->base_phys + (offset))
>> -
>>   /* Returns the dma address for reg read buffer */
>>   #define reg_buf_dma_addr(chip, vaddr) \
>>   	((chip)->reg_read_dma + \
>> @@ -454,6 +451,7 @@ struct qcom_nand_controller {
>>   struct qcom_nandc_props {
>>   	u32 ecc_modes;
>>   	u32 dev_cmd_reg_start;
>> +	u32 bam_offset;
>>   	bool supports_bam;
>>   	bool nandc_part_of_qpic;
>>   	bool qpic_version2;
>> -- 
>> 2.34.1
>>
> 

Re: [PATCH v4 1/3] mtd: rawnand: qcom: Pass 18 bit offset from NANDc base to BAM base

[Miquel Raynal]

On 25/04/2025 at 13:17:54 +0530, Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote:

> On Thu, Apr 10, 2025 at 03:30:17PM +0530, Md Sadre Alam wrote:
>> The BAM command descriptor provides only 18 bits to specify the NAND
>> register offset. 
>
> s/NAND register offset/BAM register offset
>
>> Additionally, in the BAM command descriptor, the NAND
>> register offset is supposed to be specified as "(NANDc base - BAM base)
>
> Same here.

I will fix these while applying.

Thanks,
Miquèl

[PATCH v4 2/3] mtd: rawnand: qcom: Fix last codeword read in qcom_param_page_type_exec()

[Md Sadre Alam]

For QPIC V2 onwards there is a separate register to read
last code word "QPIC_NAND_READ_LOCATION_LAST_CW_n".

qcom_param_page_type_exec() is used to read only one code word
If it configures the number of code words to 1 in QPIC_NAND_DEV0_CFG0
register then QPIC controller thinks its reading the last code word,
since we are having separate register to read the last code word,
we have to configure "QPIC_NAND_READ_LOCATION_LAST_CW_n" register
to fetch data from QPIC buffer to system memory.

Without this change page read was failing with timeout error

/ # hexdump -C /dev/mtd1
[  129.206113] qcom-nandc 1cc8000.nand-controller: failure to read page/oob
hexdump: /dev/mtd1: Connection timed out

This issue only seen on SDX targets since SDX target used QPICv2. But
same working on IPQ targets since IPQ used QPICv1.

Cc: stable@vger.kernel.org
Fixes: 89550beb098e ("mtd: rawnand: qcom: Implement exec_op()")
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Tested-by: Lakshmi Sowjanya D <quic_laksd@quicinc.com>
Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---

Change in [v4]

* No Change
 
Change in [v3]

* Updated commit header and message

* Added condition check for location register
  and location_last register based on qpic_version2

* Added Reviewed-by tag

Change in [v2]

* Updated commit message

* Added stable kernel tag

* Replaced the buf_count value of 512 with the len in bytes.

Change in [v1]

* Resolved the issue with reading a single code word in the parameter
  page read

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

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index ef2dd158ca34..a73bb154353f 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -1863,7 +1863,12 @@ static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_
 	const struct nand_op_instr *instr = NULL;
 	unsigned int op_id = 0;
 	unsigned int len = 0;
-	int ret;
+	int ret, reg_base;
+
+	reg_base = NAND_READ_LOCATION_0;
+
+	if (nandc->props->qpic_version2)
+		reg_base = NAND_READ_LOCATION_LAST_CW_0;
 
 	ret = qcom_parse_instructions(chip, subop, &q_op);
 	if (ret)
@@ -1915,7 +1920,10 @@ static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_
 	op_id = q_op.data_instr_idx;
 	len = nand_subop_get_data_len(subop, op_id);
 
-	nandc_set_read_loc(chip, 0, 0, 0, len, 1);
+	if (nandc->props->qpic_version2)
+		nandc_set_read_loc_last(chip, reg_base, 0, len, 1);
+	else
+		nandc_set_read_loc_first(chip, reg_base, 0, len, 1);
 
 	if (!nandc->props->qpic_version2) {
 		qcom_write_reg_dma(nandc, &nandc->regs->vld, NAND_DEV_CMD_VLD, 1, 0);
-- 
2.34.1

[PATCH v4 3/3] mtd: rawnand: qcom: Fix read len for onfi param page

[Md Sadre Alam]

The minimum size to fetch the data from device to QPIC buffer
is 512-bytes. If size is less than 512-bytes the data will not be
protected by ECC as per QPIC standard. So while reading onfi parameter
page from NAND device set nandc->buf_count = 512.

Cc: stable@vger.kernel.org
Fixes: 89550beb098e ("mtd: rawnand: qcom: Implement exec_op()")
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Tested-by: Lakshmi Sowjanya D <quic_laksd@quicinc.com>
Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---
Change in [v4]

* Added Reviewed-by tag

Change in [v3]

* No change

Change in [v2]

* Set buf_count to 512 in the parameter page read

Change in [v1]

 drivers/mtd/nand/raw/qcom_nandc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index a73bb154353f..1003cf118c01 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -1930,7 +1930,7 @@ static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_
 		qcom_write_reg_dma(nandc, &nandc->regs->cmd1, NAND_DEV_CMD1, 1, NAND_BAM_NEXT_SGL);
 	}
 
-	nandc->buf_count = len;
+	nandc->buf_count = 512;
 	memset(nandc->data_buffer, 0xff, nandc->buf_count);
 
 	config_nand_single_cw_page_read(chip, false, 0);
-- 
2.34.1

Re: [PATCH v4 0/3] QPIC v2 fixes for SDX75

[Miquel Raynal]

On Thu, 10 Apr 2025 15:30:16 +0530, Md Sadre Alam wrote:
> The BAM command descriptor provides only 18 bits to specify the NAND
> register offset. Additionally, in the BAM command descriptor, the NAND
> register offset is supposed to be specified as "(NANDc base - BAM base)
> + reg_off". Since, the BAM controller expecting the value in the form of
> "NANDc base - BAM base", so that added a new field 'bam_offset' in the
> NAND properties structure and use it while preparing the command descriptor.
> 
> [...]

Applied to nand/next, thanks!

[1/3] mtd: rawnand: qcom: Pass 18 bit offset from NANDc base to BAM base
      commit: ee000969f28bf579d3772bf7c0ae8aff86586e20
[2/3] mtd: rawnand: qcom: Fix last codeword read in qcom_param_page_type_exec()
      commit: 47bddabbf69da50999ec68be92b58356c687e1d6
[3/3] mtd: rawnand: qcom: Fix read len for onfi param page
      commit: e6031b11544b44966ba020c867fe438bccd3bdfa

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