Re: [PATCH v4 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Boris,

On 29/4/2020 10:48 pm, Boris Brezillon wrote:
On Wed, 29 Apr 2020 22:33:37 +0800
"Ramuthevar, Vadivel MuruganX"
<vadivel.muruganx.ramuthevar@xxxxxxxxxxxxxxx> wrote:

Hi Boris,

On 29/4/2020 10:22 pm, Boris Brezillon wrote:
On Wed, 29 Apr 2020 18:42:05 +0800
"Ramuthevar, Vadivel MuruganX"
<vadivel.muruganx.ramuthevar@xxxxxxxxxxxxxxx> wrote:
+
+#define EBU_ADDR_SEL(n)		(0x20 + (n) * 4)
+#define EBU_ADDR_MASK		(5 << 4)

It's still unclear what ADDR_MASK is for. Can you add a comment
explaining what it does?

Thank you Boris, keep review and giving inputs, will update.

Can you please explain it here before sending a new version?

Memory Region Address Mask:
Specifies the number of right-most bits in the base address that should be included in the address comparison. bits positions(7:4).

+#define EBU_ADDR_SEL_REGEN	0x1

+
+	writel(lower_32_bits(ebu_host->cs[ebu_host->cs_num].nand_pa) |
+	       EBU_ADDR_SEL_REGEN | EBU_ADDR_MASK,
+	       ebu_host->ebu + EBU_ADDR_SEL(reg));
+
+	writel(EBU_MEM_BASE_CS_0 | EBU_ADDR_MASK | EBU_ADDR_SEL_REGEN,
+	       ebu_host->ebu + EBU_ADDR_SEL(0));
+	writel(EBU_MEM_BASE_CS_1 | EBU_ADDR_MASK | EBU_ADDR_SEL_REGEN,
+	       ebu_host->ebu + EBU_ADDR_SEL(reg));

That's super weird. You seem to set EBU_ADDR_SEL(reg) twice. Are you
sure that's needed, and are we setting EBU_ADDR_SEL(0) here?

You are right, its weird only, but we need it, since different chip
select has different memory region access address.

Well, that doesn't make any sense, the second write to
EBU_ADDR_SEL(reg) overrides the first one, meaning that nand_pa is
actually never written to ADDR_SEL(reg).

it will not overwrite the first one, since two different registers
EBU_ADDR_SEL_0 EBU_ADDR_SEL  20H
EBU_ADDR_SEL_1 EBU_ADDR_SEL  24H

it is an internal address selection w.r.t chip select for nand physical address update.




Yes , we are setting both CS0 and CS1 memory access region, if you have
any concern to optimize, please suggest me, Thanks!

If you want to setup both CS, and the address written in EBU_ADDR_SEL(x)
is really related to the nand_pa address, then retrieve resources for
all CS ranges.
If it's not related, please explain what those
EBU_MEM_BASE_CS_X values encode.

Memory Region Base Address
FPI Bus addresses are compared to this base address in conjunction with the mask control(EBU_ADDR_MASK). Driver need to program this field!

Regards
Vadivel





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux