Hi Boris, Sorry for the late reply. I am busy with some other work.
> -----Original Message----- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Thursday, October 4, 2018 1:09 AM > To: Naga Sureshkumar Relli <nagas...@xilinx.com> > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org; > computersforpe...@gmail.com; marek.va...@gmail.com; Michal Simek > <mich...@xilinx.com>; linux-...@lists.infradead.org; > linux-kernel@vger.kernel.org; > nagasures...@gmail.com > Subject: Re: [LINUX PATCH v11 3/3] mtd: rawnand: arasan: Add support for > Arasan > NAND Flash Controller > > Hi Naga, > > On Tue, 25 Sep 2018 17:50:31 +0530 > Naga Sureshkumar Relli <naga.sureshkumar.re...@xilinx.com> wrote: > > > +static int anfc_read_param_get_feature_sp_read_type_exec(struct nand_chip > > *chip, > > + const struct nand_subop > > + *subop) > > +{ > > + const struct nand_op_instr *instr; > > + struct anfc_nand_controller *nfc = to_anfc(chip->controller); > > + unsigned int op_id, len; > > + struct anfc_op nfc_op = {}; > > + struct mtd_info *mtd = nand_to_mtd(chip); > > + struct anfc_nand_chip *achip = to_anfc_nand(chip); > > + u32 dma_mode, addrcycles, write_size; > > + > > + anfc_parse_instructions(chip, subop, &nfc_op); > > + instr = nfc_op.data_instr; > > + op_id = nfc_op.data_instr_idx; > > + > > + if (nfc_op.cmds[0] == NAND_CMD_PARAM) { > > + nfc->prog = PROG_RDPARAM; > > + dma_mode = 0; > > + addrcycles = 1; > > + write_size = 0; > > + } > > + if (nfc_op.cmds[0] == NAND_CMD_GET_FEATURES) { > > + nfc->prog = PROG_GET_FEATURE; > > + dma_mode = 0; > > + addrcycles = 1; > > + write_size = 0; > > + } > > + if (nfc_op.cmds[0] == NAND_CMD_READ0) { > > + nfc->prog = PROG_PGRD; > > + addrcycles = achip->raddr_cycles + achip->caddr_cycles; > > + write_size = mtd->writesize; > > + dma_mode = 1; > > + } > > + > > Sorry, but I still don't understand why nfc->prog is different. Did you try > using > PROG_PGRD for all these ops? I mean, the sequence is the same, and you keep > passing the > opcode and the number of address cycles to the engine using other reg fields. Yes, I tried it now with PROG_PGRD and I don't see any issues. I will update the same in next version of patch. Thanks for your suggestion. > > Also, you're not using the addrcycles info provided by the the address > instruction and instead > deduce it based on the opcode, which is wrong. > To make it clearer, I'd like to avoid those nfc_op.cmds[0] == NAND_OPCODE > tests, > because it's exactly the kind of things we were trying to get rid off by > introducing the - > >exec_op() interface. Ok. I understand, I will remove hardcoding the commands in the driver. And I will change the driver to read addrcycles info from address instruction. Thanks, Naga Sureshkumar Relli > > > + anfc_prepare_cmd(nfc, nfc_op.cmds[0], 0, dma_mode, write_size, > > + addrcycles); > > + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col); > > + > > + if (!nfc_op.data_instr) > > + return 0; > > + > > + len = nand_subop_get_data_len(subop, op_id); > > + anfc_rw_pio_op(mtd, nfc->buf, roundup(len, 4), 1, nfc->prog, 1, 0); > > + memcpy(instr->ctx.data.buf.in, nfc->buf, len); > > + > > + return 0; > > +}
