Hi Naga, Just a few review comments for the v12 version.
On Fri, Nov 9, 2018 at 6:00 AM Naga Sureshkumar Relli <[email protected]> wrote: > +#define PKT_OFST 0x00 > +#define PKT_CNT_SHIFT 12 > + > +#define MEM_ADDR1_OFST 0x04 > +#define MEM_ADDR2_OFST 0x08 For the sake of readability I think *_OFFSET is preferred, especially since the driver already includes short macro names. I think this is similar to the EVNT vs EVENT point. The majority of drivers in the Linux kernel do not shorten OFFSET -> OFST. > +static void anfc_rw_pio_op(struct mtd_info *mtd, u8 *buf, int len, > + bool do_read, int prog, int pktcount, int pktsize) > +{ > + struct nand_chip *chip = mtd_to_nand(mtd); > + struct anfc_nand_controller *nfc = to_anfc(chip->controller); > + struct anfc_nand_chip *achip = to_anfc_nand(chip); > + u32 *bufptr = (u32 *)buf; > + u32 cnt = 0, intr = 0; > + > + anfc_config_dma(nfc, 0); > + > + if (pktsize == 0) > + pktsize = len; > + > + anfc_setpktszcnt(nfc, pktsize, pktcount); > + > + if (!achip->strength) > + intr = MBIT_ERROR; > + > + if (do_read) > + intr |= READ_READY; > + else > + intr |= WRITE_READY; > + anfc_enable_intrs(nfc, intr); > + writel(prog, nfc->base + PROG_OFST); > + while (cnt < pktcount) { > + anfc_wait_for_event(nfc); > + cnt++; > + if (cnt == pktcount) > + anfc_enable_intrs(nfc, XFER_COMPLETE); > + if (do_read) > + ioread32_rep(nfc->base + DATA_PORT_OFST, bufptr, > + pktsize / 4); > + else > + iowrite32_rep(nfc->base + DATA_PORT_OFST, bufptr, > + pktsize / 4); > + bufptr += (pktsize / 4); > + if (cnt < pktcount) > + anfc_enable_intrs(nfc, intr); > + } > + anfc_wait_for_event(nfc); > +} Throughout the driver all calls to anfc_wait_for_event() ignores the timeout return value. It would be nice to see some error handling in case it times out - at minimum consider printing out an error message since timeout on NAND operations are fairly critical and should generally not occur. Perhaps even an argument can be made for returning -ETIMEDOUT in case of timeout. I'm putting a focus on this because I see the original non-upstream Arasan driver sometimes timeout on NAND operations when I stress test it via the UBI stress test. Not sure what the cause for the timeout is yet but either way an error message would have been helpful. Br, Martin
