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

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