Hi Boris, Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx> wrote on Sun, 10 May 2020 09:02:30 +0200: > On Fri, 8 May 2020 19:13:38 +0200 > Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote: > > > +static int anfc_len_to_steps(struct nand_chip *chip, unsigned int len) > > +{ > > + unsigned int steps = 1, pktsize = len; > > + > > + while (pktsize > ANFC_MAX_PKT_SIZE) { > > + steps *= 2; > > + pktsize = DIV_ROUND_UP(len, steps); > > + } > > > Same here, you shouldn't have a round_up() but instead complain if > "len != pkt_size * steps" > > if (len % 4) > return -ENOTSUPP; > > if (len < ANFC_MAX_PKT_SIZE) > return len; > > for (steps = 2; steps < ANFC_MAX_STEPS; steps *= 2) { > pkt_size = len / steps; > if (pkt_size <= ANFC_MAX_PKT_SIZE) > break; > } > > if (pkt_size * steps != len) > return -ENOTSUPP; > > return pkt_size; > > > + > > + if (steps > ANFC_MAX_STEPS) > > + return -ENOTSUPP; > > + > > + return steps; > > +} I took the logic of the above proposal and extended the helper to be "anfc_pkt_len_config", taking two pointers as argument: *steps and *pktsize, which will be updated in case of success. Otherwise this function returns an error and can be added to the "check_op" path instead of only failing at execution time. ---8<--- +static int anfc_pkt_len_config(unsigned int len, unsigned int *steps, + unsigned int *pktsize) +{ + unsigned int nb, sz; + + for (nb = 1; nb < ANFC_MAX_STEPS; nb *= 2) { + sz = len / nb; + if (sz <= ANFC_MAX_PKT_SIZE) + break; + } + + if (sz * nb != len) + return -ENOTSUPP; + + if (steps) + *steps = nb; + + if (pktsize) + *pktsize = sz; + + return 0; +} --->8--- And then, in anfc_check_op(): + case NAND_OP_DATA_IN_INSTR: + case NAND_OP_DATA_OUT_INSTR: + if (instr->ctx.data.len > ANFC_MAX_CHUNK_SIZE) + return -ENOTSUPP; + + if (anfc_pkt_len_config(instr->ctx.data.len NULL, NULL)) + return -ENOTSUPP; + + break; ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/