Hi Sricharan, quic_srichara@xxxxxxxxxxx wrote on Tue, 8 Aug 2023 00:24:08 +0530: > Hi Mani/Miquel, > > On 8/6/2023 1:28 PM, Sricharan Ramabadhran wrote: > > Hi Miquel/Mani/Dan, > > > > <..> > > > >>>>> The || should be &&, otherwise it cannot work, or am I missing > >>>>> something? > >>>> > >>>> Yeah. That's how this bug normally looks like. NAND_OP_CMD_INSTR > >>>> always returns -ENOTSUPP. > >>>> > >> > >> Yes, this is a bug. > >> > >> I didn't get a chance to review the exec_op conversion series (blame >> myself). > >> Now I see scope for a cleaup series :/ > >> > >> Let me spin something by the end of today. > >> > > Thanks a lot Mani for the quick fixes. Will test it. > > Sorry it was a holiday for us past few days, so just checked this. > > > > Regards, > > Sricharan > > With this series applied on linux-next, started seeing the below > messages flooded on console while doing mtd r/w. > "xxx "Opcode not supported: 0" > > opcode '0' corresponds to NAND_CMD_READ0. This command inturn was > invoked from qcom_nandc.c driver from below places. For read/write_page > driver does not use the exec ops. Hence these calls just ends up > being -ENOTSUPP and ignored. So removed their invocations. > If this is fine, can this be added to your series ? (will send > git-format patch to add to your cleanup series). So far, tested > mtd raw/block read/writes and all works fine. Will do further tests > as well. Unless I really don't understand the controller, this is non sense. nand_read_page_op() is precisely what allows your NAND to perform a read. Removing this call cannot work. What you need is a proper ->exec_op() implementation, and repeating this becomes slightly annoying. Look at your qcom_op_cmd_mapping, you don't even have a path for reads. I bet something along: CMD_READ0: ret = XXX_OPCODE_READ; break; will make it work. Please fix the driver and test with nandbiterrs -i. If this test works, it is encouraging. Otherwise it is still broken. > --- a/drivers/mtd/nand/raw/qcom_nandc.c > +++ b/drivers/mtd/nand/raw/qcom_nandc.c > @@ -1470,7 +1470,6 @@ qcom_nandc_read_cw_raw(struct mtd_info *mtd, struct nand_chip *chip, > int ret, reg_off = FLASH_BUF_ACC, read_loc = 0; > int raw_cw = cw; > > - nand_read_page_op(chip, page, 0, NULL, 0); > host->use_ecc = false; > > if (nandc->props->qpic_v2) > @@ -1890,7 +1889,6 @@ static int qcom_nandc_read_page(struct nand_chip *chip, u8 *buf, > if (host->nr_boot_partitions) > qcom_nandc_codeword_fixup(host, page); > > - nand_read_page_op(chip, page, 0, NULL, 0); > nandc->buf_count = 0; > nandc->buf_start = 0; > host->use_ecc = true; > @@ -1965,8 +1963,6 @@ static int qcom_nandc_write_page(struct nand_chip *chip, const u8 *buf, > if (host->nr_boot_partitions) > qcom_nandc_codeword_fixup(host, page); > > - nand_prog_page_begin_op(chip, page, 0, NULL, 0); > - > set_address(host, 0, page); > nandc->buf_count = 0; > nandc->buf_start = 0; > @@ -2039,7 +2035,6 @@ static int qcom_nandc_write_page_raw(struct nand_chip *chip, > if (host->nr_boot_partitions) > qcom_nandc_codeword_fixup(host, page); > > - nand_prog_page_begin_op(chip, page, 0, NULL, 0); > clear_read_regs(nandc); > clear_bam_transaction(nandc); > > > Regards, > Sricharan Miquèl
