Re: [PATCH 04/11] nand: spi: Add read function support

Boris Brezillon
2017-02-21T11:02:47Z

On Tue, 21 Feb 2017 18:06:03 +0800
Peter Pan <peterpansjtu@gmail.com> wrote:

> >> +/**
> >> + * spi_nand_transfer_oob - transfer oob to client buffer
> >> + * @chip: SPI-NAND device structure
> >> + * @oob: oob destination address
> >> + * @ops: oob ops structure
> >> + * @len: size of oob to transfer
> >> + */
> >> +static void spi_nand_transfer_oob(struct spi_nand_chip *chip, u8 *oob,
> >> + struct mtd_oob_ops *ops, size_t len)
> >> +{
> >> + struct mtd_info *mtd = spi_nand_to_mtd(chip);
> >> + int ret;
> >> +
> >> + switch (ops->mode) {
> >> +
> >> + case MTD_OPS_PLACE_OOB:
> >> + case MTD_OPS_RAW:
> >> + memcpy(oob, chip->oobbuf + ops->ooboffs, len);
> >> + return;
> >> +
> >> + case MTD_OPS_AUTO_OOB:
> >> + ret = mtd_ooblayout_get_databytes(mtd, oob, chip->oobbuf,
> >> + ops->ooboffs, len);
> >> + BUG_ON(ret);
> >> + return;
> >> +
> >> + default:
> >> + BUG();
> >
> > Let's try to avoid these BUG_ON() calls. If you want transfer_oob() to
> > report an error, change the function prototype and check the ret code
> > in the caller.
>
> I will remove the BUG_ON()s in v2. Boris, I think this part can be put in
> new NAND core, what do you think?

Well, assuming we move the ->oob buffer in nand_device struct, then
yes. That's exactly for this reason I wanted to see a version of the
SPI NAND framework before merging the "interface-agnostic NAND layer".
I guess we'll find other candidates for factorization, but I also think
we should not try to get the perfect implementation before merging
something (IOW, let's keep this improvement on our TODO list).

>
> >
> >> + }
> >> +}