[PATCH v3 0/4] QPIC v2 fixes for SDX75

[PATCH v3 0/4] QPIC v2 fixes for SDX75

[Md Sadre Alam]

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 (4):
  mtd: rawnand: qcom: Pass 18 bit offset from QPIC base address to BAM
  mtd: rawnand: qcom: Fix last codeword read in
    qcom_param_page_type_exec()
  mtd: rawnand: qcom: Fix read len for onfi param page
  spi: spi-qpic-snand: set nandc_offset for ipq9574

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

-- 
2.34.1


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

[PATCH v3 1/4] mtd: rawnand: qcom: Pass 18 bit offset from QPIC base address to BAM

[Md Sadre Alam]

Currently we are configuring lower 24 bits of address in descriptor
whereas QPIC design expects 18 bit register offset from QPIC base
address to be configured in cmd descriptors. This is leading to a
different address actually being used in HW, leading to wrong value
read.

the actual issue is that the NANDc base address is different from the
QPIC base address. But the driver doesn't take it into account and just
used the QPIC base as the NANDc base. This used to work as the NANDc IP
only considers the lower 18 bits of the address passed by the driver to
derive the register offset. Since the base address of QPIC used to contain
all 0 for lower 18 bits (like 0x07980000), the driver ended up passing the
actual register offset in it and NANDc worked properly. But on newer SoCs
like SDX75, the QPIC base address doesn't contain all 0 for lower 18 bits
(like 0x01C98000). So NANDc sees wrong offset as per the current logic

The address should be passed to BAM 0x30000 + offset. In older targets
the lower 18-bits are zero so that correct address being paased. But
in newer targets the lower 18-bits are non-zero in QPIC base so that
0x300000 + offset giving the wrong value.

SDX75 : QPIC_QPIC | 0x01C98000 (Lower 18 bits are non zero)
SDX55 : QPIC_QPIC | 0x1B00000 (Lower 18 bits are zero) Same for
older targets.

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>
---

Change in [v3]

* Updated commit message
* Removed dev_cmd_reg_start = 0 , which
  was wrongely got added

Change in [v2]

* Updated commit message

* Added Fixes tag

* Added stable kernel tag

* Renamed the variable from offset_from_qpic to nandc_offset

Change in [v1]

* Preliminary correction for the register address forwarded to BAM

 drivers/mtd/nand/raw/qcom_nandc.c    | 4 ++++
 include/linux/mtd/nand-qpic-common.h | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 5eaa0be367cd..5443cb918e0b 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,
+	.nandc_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,
+	.nandc_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,
+	.nandc_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,
+	.nandc_offset = 0x30000,
 };
 
 /*
diff --git a/include/linux/mtd/nand-qpic-common.h b/include/linux/mtd/nand-qpic-common.h
index cd7172e6c1bb..6268f08b9d19 100644
--- a/include/linux/mtd/nand-qpic-common.h
+++ b/include/linux/mtd/nand-qpic-common.h
@@ -200,7 +200,7 @@
 #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))
+#define nandc_reg_phys(chip, offset)  ((nandc)->props->nandc_offset + (offset))
 
 /* Returns the dma address for reg read buffer */
 #define reg_buf_dma_addr(chip, vaddr) \
@@ -458,6 +458,7 @@ struct qcom_nandc_props {
 	bool nandc_part_of_qpic;
 	bool qpic_version2;
 	bool use_codeword_fixup;
+	u32 nandc_offset;
 };
 
 void qcom_free_bam_transaction(struct qcom_nand_controller *nandc);
-- 
2.34.1


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

Re: [PATCH v3 1/4] mtd: rawnand: qcom: Pass 18 bit offset from QPIC base address to BAM

[Manivannan Sadhasivam]

On Mon, Mar 10, 2025 at 05:39:03PM +0530, Md Sadre Alam wrote:
> Currently we are configuring lower 24 bits of address in descriptor
> whereas QPIC design expects 18 bit register offset from QPIC base
> address to be configured in cmd descriptors. This is leading to a
> different address actually being used in HW, leading to wrong value
> read.
> 
> the actual issue is that the NANDc base address is different from the
> QPIC base address. But the driver doesn't take it into account and just
> used the QPIC base as the NANDc base. This used to work as the NANDc IP
> only considers the lower 18 bits of the address passed by the driver to
> derive the register offset. Since the base address of QPIC used to contain
> all 0 for lower 18 bits (like 0x07980000), the driver ended up passing the

What is this address? Is it coming from DT?

> actual register offset in it and NANDc worked properly. But on newer SoCs
> like SDX75, the QPIC base address doesn't contain all 0 for lower 18 bits
> (like 0x01C98000). So NANDc sees wrong offset as per the current logic
> 
> The address should be passed to BAM 0x30000 + offset. In older targets

You gave no explanation on how this 0x30000 offset came into picture. I gave the
reasoning in v2:

"SDX55's NANDc base is 0x01b30000 and it has bits 17 and 18 set corresponding to
0x30000. So it is correct that the IP only considers lower 18 bits and it used
to work as the driver ended up passing 0x3000 + register offset."

Then you replied:

"This address 0x30000 is the address from QPIC_BASE to QPIC_EBI2NAND
e.g for SDX55 and SDX65 the QPIC_BASE is 0x01B00000. So here lower 18-bits
are zero only."

No one outside Qcom knows what QPIC_BASE and QPIC_EBI2NAND are. We just know the
NANDc address mentioned in DT, which corresponds to 0x01b30000 for SDX55.

Please reword the commit message to present the full picture and not half baked
info. This is v3, I see no improvement in the commit message, sorry.

> the lower 18-bits are zero so that correct address being paased. But
> in newer targets the lower 18-bits are non-zero in QPIC base so that
> 0x300000 + offset giving the wrong value.
> 
> SDX75 : QPIC_QPIC | 0x01C98000 (Lower 18 bits are non zero)
> SDX55 : QPIC_QPIC | 0x1B00000 (Lower 18 bits are zero) Same for

There is no address as '0x1B00000' in DT.

- Mani

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

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

Re: [PATCH v3 1/4] mtd: rawnand: qcom: Pass 18 bit offset from QPIC base address to BAM

[Md Sadre Alam]

On 3/18/2025 1:03 PM, Manivannan Sadhasivam wrote:
> On Mon, Mar 10, 2025 at 05:39:03PM +0530, Md Sadre Alam wrote:
>> Currently we are configuring lower 24 bits of address in descriptor
>> whereas QPIC design expects 18 bit register offset from QPIC base
>> address to be configured in cmd descriptors. This is leading to a
>> different address actually being used in HW, leading to wrong value
>> read.
>>
>> the actual issue is that the NANDc base address is different from the
>> QPIC base address. But the driver doesn't take it into account and just
>> used the QPIC base as the NANDc base. This used to work as the NANDc IP
>> only considers the lower 18 bits of the address passed by the driver to
>> derive the register offset. Since the base address of QPIC used to contain
>> all 0 for lower 18 bits (like 0x07980000), the driver ended up passing the
> 
> What is this address? Is it coming from DT?
> 
>> actual register offset in it and NANDc worked properly. But on newer SoCs
>> like SDX75, the QPIC base address doesn't contain all 0 for lower 18 bits
>> (like 0x01C98000). So NANDc sees wrong offset as per the current logic
>>
>> The address should be passed to BAM 0x30000 + offset. In older targets
> 
> You gave no explanation on how this 0x30000 offset came into picture. I gave the
> reasoning in v2:
> 
> "SDX55's NANDc base is 0x01b30000 and it has bits 17 and 18 set corresponding to
> 0x30000. So it is correct that the IP only considers lower 18 bits and it used
> to work as the driver ended up passing 0x3000 + register offset."
> 
> Then you replied:
> 
> "This address 0x30000 is the address from QPIC_BASE to QPIC_EBI2NAND
> e.g for SDX55 and SDX65 the QPIC_BASE is 0x01B00000. So here lower 18-bits
> are zero only."
> 
> No one outside Qcom knows what QPIC_BASE and QPIC_EBI2NAND are. We just know the
> NANDc address mentioned in DT, which corresponds to 0x01b30000 for SDX55.
> 
> Please reword the commit message to present the full picture and not half baked
> info. This is v3, I see no improvement in the commit message, sorry.
> 
>> the lower 18-bits are zero so that correct address being paased. But
>> in newer targets the lower 18-bits are non-zero in QPIC base so that
>> 0x300000 + offset giving the wrong value.
>>
>> SDX75 : QPIC_QPIC | 0x01C98000 (Lower 18 bits are non zero)
>> SDX55 : QPIC_QPIC | 0x1B00000 (Lower 18 bits are zero) Same for
> 
> There is no address as '0x1B00000' in DT.

Mani,

Please see if this commit message would be acceptable?

	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 offset - BAM base offset) + reg_off".
	Since, the nand driver isn't aware of the BAM base offset,
	have the value of "NANDc offset - BAM base offset" in a new
	field 'nandc_offset' in the NAND properties structure and use
	it while preparing the descriptor.

	Previously, the NAND driver was incorrectly specifying the
	NAND register offset directly in the BAM descriptor.

Thanks
Alam

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

Re: [PATCH v3 1/4] mtd: rawnand: qcom: Pass 18 bit offset from QPIC base address to BAM

[Manivannan Sadhasivam]

On Thu, Mar 20, 2025 at 11:23:40AM +0530, Md Sadre Alam wrote:
> 
> 
> On 3/18/2025 1:03 PM, Manivannan Sadhasivam wrote:
> > On Mon, Mar 10, 2025 at 05:39:03PM +0530, Md Sadre Alam wrote:
> > > Currently we are configuring lower 24 bits of address in descriptor
> > > whereas QPIC design expects 18 bit register offset from QPIC base
> > > address to be configured in cmd descriptors. This is leading to a
> > > different address actually being used in HW, leading to wrong value
> > > read.
> > > 
> > > the actual issue is that the NANDc base address is different from the
> > > QPIC base address. But the driver doesn't take it into account and just
> > > used the QPIC base as the NANDc base. This used to work as the NANDc IP
> > > only considers the lower 18 bits of the address passed by the driver to
> > > derive the register offset. Since the base address of QPIC used to contain
> > > all 0 for lower 18 bits (like 0x07980000), the driver ended up passing the
> > 
> > What is this address? Is it coming from DT?
> > 
> > > actual register offset in it and NANDc worked properly. But on newer SoCs
> > > like SDX75, the QPIC base address doesn't contain all 0 for lower 18 bits
> > > (like 0x01C98000). So NANDc sees wrong offset as per the current logic
> > > 
> > > The address should be passed to BAM 0x30000 + offset. In older targets
> > 
> > You gave no explanation on how this 0x30000 offset came into picture. I gave the
> > reasoning in v2:
> > 
> > "SDX55's NANDc base is 0x01b30000 and it has bits 17 and 18 set corresponding to
> > 0x30000. So it is correct that the IP only considers lower 18 bits and it used
> > to work as the driver ended up passing 0x3000 + register offset."
> > 
> > Then you replied:
> > 
> > "This address 0x30000 is the address from QPIC_BASE to QPIC_EBI2NAND
> > e.g for SDX55 and SDX65 the QPIC_BASE is 0x01B00000. So here lower 18-bits
> > are zero only."
> > 
> > No one outside Qcom knows what QPIC_BASE and QPIC_EBI2NAND are. We just know the
> > NANDc address mentioned in DT, which corresponds to 0x01b30000 for SDX55.
> > 
> > Please reword the commit message to present the full picture and not half baked
> > info. This is v3, I see no improvement in the commit message, sorry.
> > 
> > > the lower 18-bits are zero so that correct address being paased. But
> > > in newer targets the lower 18-bits are non-zero in QPIC base so that
> > > 0x300000 + offset giving the wrong value.
> > > 
> > > SDX75 : QPIC_QPIC | 0x01C98000 (Lower 18 bits are non zero)
> > > SDX55 : QPIC_QPIC | 0x1B00000 (Lower 18 bits are zero) Same for
> > 
> > There is no address as '0x1B00000' in DT.
> 
> Mani,
> 
> Please see if this commit message would be acceptable?
> 
> 	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 offset - BAM base offset) + reg_off".

Isn't it, (NANDc base - BAM base)? 'offset' is not valid here.

And also, you are just mixing the names everywhere. Here you say, NANDc base
and BAM base, but in patch 4:

"NAND register addresses to be computed based on the NAND register offset from
QPIC base". So the second address is BAM or QPIC?

Please be consistent with naming.

> 	Since, the nand driver isn't aware of the BAM base offset,
> 	have the value of "NANDc offset - BAM base offset" in a new
> 	field 'nandc_offset' in the NAND properties structure and use
> 	it while preparing the descriptor.
> 

And what about 'nandc_offset'? NANDc is already the term used for NAND
controller which has the base address of 0x01b30000 in DT. So clearly, the name
of the offset variable is not correct.

> 	Previously, the NAND driver was incorrectly specifying the
> 	NAND register offset directly in the BAM descriptor.
> 

No. Previously, the driver was specifying the NANDc base address in the BAM
descriptor. You are now trying to pass the register offset.

- Mani

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

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

Re: [PATCH v3 1/4] mtd: rawnand: qcom: Pass 18 bit offset from QPIC base address to BAM

[Gabor Juhos]

2025. 03. 10. 13:09 keltezéssel, Md Sadre Alam írta:
> Currently we are configuring lower 24 bits of address in descriptor
> whereas QPIC design expects 18 bit register offset from QPIC base
> address to be configured in cmd descriptors. This is leading to a
> different address actually being used in HW, leading to wrong value
> read.
> 
> the actual issue is that the NANDc base address is different from the
> QPIC base address. But the driver doesn't take it into account and just
> used the QPIC base as the NANDc base. This used to work as the NANDc IP
> only considers the lower 18 bits of the address passed by the driver to
> derive the register offset. Since the base address of QPIC used to contain
> all 0 for lower 18 bits (like 0x07980000), the driver ended up passing the
> actual register offset in it and NANDc worked properly. But on newer SoCs
> like SDX75, the QPIC base address doesn't contain all 0 for lower 18 bits
> (like 0x01C98000). So NANDc sees wrong offset as per the current logic
> 
> The address should be passed to BAM 0x30000 + offset. In older targets
> the lower 18-bits are zero so that correct address being paased. But
> in newer targets the lower 18-bits are non-zero in QPIC base so that
> 0x300000 + offset giving the wrong value.
> 
> SDX75 : QPIC_QPIC | 0x01C98000 (Lower 18 bits are non zero)
> SDX55 : QPIC_QPIC | 0x1B00000 (Lower 18 bits are zero) Same for
> older targets.
> 
> 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>
> ---

<...>

>  /*
> diff --git a/include/linux/mtd/nand-qpic-common.h b/include/linux/mtd/nand-qpic-common.h
> index cd7172e6c1bb..6268f08b9d19 100644
> --- a/include/linux/mtd/nand-qpic-common.h
> +++ b/include/linux/mtd/nand-qpic-common.h
> @@ -200,7 +200,7 @@
>  #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))
> +#define nandc_reg_phys(chip, offset)  ((nandc)->props->nandc_offset + (offset))

The macro has no parameter named 'nandc', so this works only when there is an
identifier with that name in the code where the macro is used.

Additionally, the macro will no longer return the physical address of a register
after the change, so both the comment before the macro and the name of the macro
will be misleading.

Since the macro is used only in the qcom_prep_bam_dma_desc_cmd() function to
compute the 'addr' parameter for the bam_prep_ce{_le32}() functions, maybe it
would be better to get rid of it completely, and do the computation directly in
the function instead.

Regards,
Gabor

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

Re: [PATCH v3 1/4] mtd: rawnand: qcom: Pass 18 bit offset from QPIC base address to BAM

[Md Sadre Alam]

On 3/18/2025 8:22 PM, Gabor Juhos wrote:
> 2025. 03. 10. 13:09 keltezéssel, Md Sadre Alam írta:
>> Currently we are configuring lower 24 bits of address in descriptor
>> whereas QPIC design expects 18 bit register offset from QPIC base
>> address to be configured in cmd descriptors. This is leading to a
>> different address actually being used in HW, leading to wrong value
>> read.
>>
>> the actual issue is that the NANDc base address is different from the
>> QPIC base address. But the driver doesn't take it into account and just
>> used the QPIC base as the NANDc base. This used to work as the NANDc IP
>> only considers the lower 18 bits of the address passed by the driver to
>> derive the register offset. Since the base address of QPIC used to contain
>> all 0 for lower 18 bits (like 0x07980000), the driver ended up passing the
>> actual register offset in it and NANDc worked properly. But on newer SoCs
>> like SDX75, the QPIC base address doesn't contain all 0 for lower 18 bits
>> (like 0x01C98000). So NANDc sees wrong offset as per the current logic
>>
>> The address should be passed to BAM 0x30000 + offset. In older targets
>> the lower 18-bits are zero so that correct address being paased. But
>> in newer targets the lower 18-bits are non-zero in QPIC base so that
>> 0x300000 + offset giving the wrong value.
>>
>> SDX75 : QPIC_QPIC | 0x01C98000 (Lower 18 bits are non zero)
>> SDX55 : QPIC_QPIC | 0x1B00000 (Lower 18 bits are zero) Same for
>> older targets.
>>
>> 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>
>> ---
> 
> <...>
> 
>>   /*
>> diff --git a/include/linux/mtd/nand-qpic-common.h b/include/linux/mtd/nand-qpic-common.h
>> index cd7172e6c1bb..6268f08b9d19 100644
>> --- a/include/linux/mtd/nand-qpic-common.h
>> +++ b/include/linux/mtd/nand-qpic-common.h
>> @@ -200,7 +200,7 @@
>>   #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))
>> +#define nandc_reg_phys(chip, offset)  ((nandc)->props->nandc_offset + (offset))
> 
> The macro has no parameter named 'nandc', so this works only when there is an
> identifier with that name in the code where the macro is used.
> 
> Additionally, the macro will no longer return the physical address of a register
> after the change, so both the comment before the macro and the name of the macro
> will be misleading.
> 
> Since the macro is used only in the qcom_prep_bam_dma_desc_cmd() function to
> compute the 'addr' parameter for the bam_prep_ce{_le32}() functions, maybe it
> would be better to get rid of it completely, and do the computation directly in
> the function instead.
Ok, Will handle in next revision.
> 
> Regards,
> Gabor

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

[PATCH v3 2/4] 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 [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 5443cb918e0b..d41c0e3926ed 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


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

[PATCH v3 3/4] 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()")
Tested-by: Lakshmi Sowjanya D <quic_laksd@quicinc.com>
Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---

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 d41c0e3926ed..c3fdafb7e9eb 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


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

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

[Manivannan Sadhasivam]

On Mon, Mar 10, 2025 at 05:39:05PM +0530, Md Sadre Alam wrote:
> 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()")
> 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

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

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

[PATCH v3 4/4] spi: spi-qpic-snand: set nandc_offset for ipq9574

[Md Sadre Alam]

The BAM block expects NAND register addresses to be computed based on
the NAND register offset from QPIC base. This value is 0x30000 for
ipq9574. Update the 'nandc_offset' value in the qcom_nandc_props
appropriately.

Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---

Change in [v3]

* Added nand_offset for proper address calculation
  for newer Socs

Change in [v2]

* This patch was not part of v2

Change in [v1]

* This patch was not part of v1

 drivers/spi/spi-qpic-snand.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-qpic-snand.c b/drivers/spi/spi-qpic-snand.c
index 8c413a6a5152..85a742e21cf9 100644
--- a/drivers/spi/spi-qpic-snand.c
+++ b/drivers/spi/spi-qpic-snand.c
@@ -1604,6 +1604,7 @@ static void qcom_spi_remove(struct platform_device *pdev)
 static const struct qcom_nandc_props ipq9574_snandc_props = {
 	.dev_cmd_reg_start = 0x7000,
 	.supports_bam = true,
+	.nandc_offset = 0x30000,
 };
 
 static const struct of_device_id qcom_snandc_of_match[] = {
-- 
2.34.1


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

Re: [PATCH v3 4/4] spi: spi-qpic-snand: set nandc_offset for ipq9574

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 331 bytes --]

On Mon, Mar 10, 2025 at 05:39:06PM +0530, Md Sadre Alam wrote:
> The BAM block expects NAND register addresses to be computed based on
> the NAND register offset from QPIC base. This value is 0x30000 for
> ipq9574. Update the 'nandc_offset' value in the qcom_nandc_props
> appropriately.

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

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

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

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

Re: [PATCH v3 4/4] spi: spi-qpic-snand: set nandc_offset for ipq9574

[Manivannan Sadhasivam]

On Mon, Mar 10, 2025 at 05:39:06PM +0530, Md Sadre Alam wrote:
> The BAM block expects NAND register addresses to be computed based on
> the NAND register offset from QPIC base. This value is 0x30000 for
> ipq9574. Update the 'nandc_offset' value in the qcom_nandc_props
> appropriately.
> 
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>

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

- Mani

> ---
> 
> Change in [v3]
> 
> * Added nand_offset for proper address calculation
>   for newer Socs
> 
> Change in [v2]
> 
> * This patch was not part of v2
> 
> Change in [v1]
> 
> * This patch was not part of v1
> 
>  drivers/spi/spi-qpic-snand.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/spi/spi-qpic-snand.c b/drivers/spi/spi-qpic-snand.c
> index 8c413a6a5152..85a742e21cf9 100644
> --- a/drivers/spi/spi-qpic-snand.c
> +++ b/drivers/spi/spi-qpic-snand.c
> @@ -1604,6 +1604,7 @@ static void qcom_spi_remove(struct platform_device *pdev)
>  static const struct qcom_nandc_props ipq9574_snandc_props = {
>  	.dev_cmd_reg_start = 0x7000,
>  	.supports_bam = true,
> +	.nandc_offset = 0x30000,
>  };
>  
>  static const struct of_device_id qcom_snandc_of_match[] = {
> -- 
> 2.34.1
> 

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

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

Re: [PATCH v3 4/4] spi: spi-qpic-snand: set nandc_offset for ipq9574

[Gabor Juhos]

2025. 03. 10. 13:09 keltezéssel, Md Sadre Alam írta:
> The BAM block expects NAND register addresses to be computed based on
> the NAND register offset from QPIC base. This value is 0x30000 for
> ipq9574. Update the 'nandc_offset' value in the qcom_nandc_props
> appropriately.
> 
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> ---
> 
> Change in [v3]
> 
> * Added nand_offset for proper address calculation
>   for newer Socs
> 
> Change in [v2]
> 
> * This patch was not part of v2
> 
> Change in [v1]
> 
> * This patch was not part of v1
> 
>  drivers/spi/spi-qpic-snand.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/spi/spi-qpic-snand.c b/drivers/spi/spi-qpic-snand.c
> index 8c413a6a5152..85a742e21cf9 100644
> --- a/drivers/spi/spi-qpic-snand.c
> +++ b/drivers/spi/spi-qpic-snand.c
> @@ -1604,6 +1604,7 @@ static void qcom_spi_remove(struct platform_device *pdev)
>  static const struct qcom_nandc_props ipq9574_snandc_props = {
>  	.dev_cmd_reg_start = 0x7000,
>  	.supports_bam = true,
> +	.nandc_offset = 0x30000,
>  };

Applying the first patch alone results in the following error on IPQ9574:

[    3.596403] qcom_snand 79b0000.spi: failure in submitting cmd descriptor
[    3.596490] spi-nand spi0.0: probe with driver spi-nand failed with error -110

So this change should be integrated into the first patch. Otherwise, SPI NAND
support would be broken on IPQ9574 temporarily between the first and the
current patch.

Regards,
Gabor

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

Re: [PATCH v3 4/4] spi: spi-qpic-snand: set nandc_offset for ipq9574

[Md Sadre Alam]

On 3/18/2025 8:28 PM, Gabor Juhos wrote:
> 2025. 03. 10. 13:09 keltezéssel, Md Sadre Alam írta:
>> The BAM block expects NAND register addresses to be computed based on
>> the NAND register offset from QPIC base. This value is 0x30000 for
>> ipq9574. Update the 'nandc_offset' value in the qcom_nandc_props
>> appropriately.
>>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> ---
>>
>> Change in [v3]
>>
>> * Added nand_offset for proper address calculation
>>    for newer Socs
>>
>> Change in [v2]
>>
>> * This patch was not part of v2
>>
>> Change in [v1]
>>
>> * This patch was not part of v1
>>
>>   drivers/spi/spi-qpic-snand.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/spi/spi-qpic-snand.c b/drivers/spi/spi-qpic-snand.c
>> index 8c413a6a5152..85a742e21cf9 100644
>> --- a/drivers/spi/spi-qpic-snand.c
>> +++ b/drivers/spi/spi-qpic-snand.c
>> @@ -1604,6 +1604,7 @@ static void qcom_spi_remove(struct platform_device *pdev)
>>   static const struct qcom_nandc_props ipq9574_snandc_props = {
>>   	.dev_cmd_reg_start = 0x7000,
>>   	.supports_bam = true,
>> +	.nandc_offset = 0x30000,
>>   };
> 
> Applying the first patch alone results in the following error on IPQ9574:
> 
> [    3.596403] qcom_snand 79b0000.spi: failure in submitting cmd descriptor
> [    3.596490] spi-nand spi0.0: probe with driver spi-nand failed with error -110
> 
> So this change should be integrated into the first patch. Otherwise, SPI NAND
> support would be broken on IPQ9574 temporarily between the first and the
> current patch.
Ok, will integrate this in first patch in next revision.
> 
> Regards,
> Gabor

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

Re: [PATCH v3 0/4] QPIC v2 fixes for SDX75

[Manivannan Sadhasivam]

On Mon, Mar 10, 2025 at 05:39:02PM +0530, Md Sadre Alam wrote:

Nothing in the cover letter to describe the series other than the changelog?

Btw, the driver has been broken for almost 9 releases and the fix series is
pending for 4 months. This tells a lot about the commitment to fix the bugs :/

- Mani

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

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