On 11/21/2024 12:29 PM, Manivannan Sadhasivam wrote:
On Thu, Nov 21, 2024 at 11:33:13AM +0530, Md Sadre Alam wrote:
On 11/20/2024 12:31 PM, Manivannan Sadhasivam wrote:
On Tue, Nov 19, 2024 at 02:50:57PM +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
You mean 'QPIC IP' here? But is it QPIC or NANDc? I guess the later.
It's QPIC IP only.
Hmm, so what is the difference between QPIC and NANDc?
QPIC is wrapper which integrates NANDc. So only QPIC (Qualcomm Parallel
Interface Controller) will be exposed for interface.
address to be configured in cmd descriptors. This is leading to a
different address actually being used in HW, leading to wrong value
read.
This doesn't clearly say what the actual issue is. IIUC, the 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.
Yes correct. If QPIC address = 0x07980000 and QPIC_EBI2NAND address = 0x079b0000
the the diff is 0x30000, this is the actual offset expected by QPIC RTL code.
and RTL needs only 18-bit offset.
Okay. So the driver used to pass 0x30000 + offset in older targets and on newer
ones starting from SDX75, 0x30000 is not passed correctly due to the changed
QPIC base address.
Yes, correct. In SDX75 the first 18-bits of QPIC base address are non-zero.
Please mention it clearly in description.
Ok
- Mani
Older targets also used same configuration (lower 24 bits) like sdxpinn,
Please use actual product names and not internal names. I believe you are
referring to SDX55/SDX65 here.
Ok , will change in next revision.
ipq etc. but issue is masked in older targets due to lower 18 bits of QPIC
base address being zero leading to expected address generation.
Sdxpinn : QPIC_QPIC | 0x01C98000 (Lower 18 bits are non zero)
Sdxnightjar : QPIC_QPIC | 0x07980000 (Lower 18 bits are zero) Same for
older targets.
Same here.
Ok
Signed-off-by: Md Sadre Alam <quic_mdalam@xxxxxxxxxxx>
Please add relevant Fixes tag.
Ok
---
drivers/mtd/nand/raw/qcom_nandc.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index b8cff9240b28..34ee8555fb8a 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -207,7 +207,7 @@ nandc_set_reg(chip, reg, \
#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->offset_from_qpic + (offset))
/* Returns the dma address for reg read buffer */
#define reg_buf_dma_addr(chip, vaddr) \
@@ -561,6 +561,7 @@ struct qcom_nandc_props {
bool is_qpic;
bool qpic_v2;
bool use_codeword_fixup;
+ u32 offset_from_qpic;
nandc_offset?
Ok
};
/* Frees the BAM transaction memory */
@@ -3477,6 +3478,7 @@ static const struct qcom_nandc_props ipq806x_nandc_props = {
.is_bam = false,
.use_codeword_fixup = true,
.dev_cmd_reg_start = 0x0,
+ .offset_from_qpic = 0x30000,
How 0x30000 is supposed to work? You said the NANDc ignores lower 18 bits, but
this has 17th and 18th bits set.
Not this address 0x30000, this the diff b/w QPIC base and EBI2NAND base. The 18-bits we have see
on this address 0x07980000 and this address 0x01C98000.
- Mani
