Re: [PATCH v3 24/42] mtd: nand: add support for ts72xx

From: Miquel Raynal
Date: Mon Jul 24 2023 - 03:09:24 EST


Hi Andy,

> > +static int ts72xx_nand_attach_chip(struct nand_chip *chip)
> > +{
> > + switch (chip->ecc.engine_type) {
> > + case NAND_ECC_ENGINE_TYPE_SOFT:
> > + if (chip->ecc.algo == NAND_ECC_ALGO_UNKNOWN)
> > + chip->ecc.algo = NAND_ECC_ALGO_HAMMING;
> > + break;
> > + case NAND_ECC_ENGINE_TYPE_ON_HOST:
> > + return -EINVAL;
> > + default:
>
> > + break;
>
> Here it will return 0, is it a problem?

Seems ok, there are two other situations: on-die ECC engine and no ECC
engine, both do not require any specific handling on the controller
side.

>
> > + }
> > +
> > + return 0;
> > +}
>
> ...
>
> > +static void ts72xx_nand_remove(struct platform_device *pdev)
> > +{
> > + struct ts72xx_nand_data *data = platform_get_drvdata(pdev);
> > + struct nand_chip *chip = &data->chip;
> > + int ret;
> > +
> > + ret = mtd_device_unregister(nand_to_mtd(chip));
>
> > + WARN_ON(ret);
>
> Why?! Is it like this in other MTD drivers?

Yes, we did not yet change the internal machinery to return void, and
we don't want people to think getting errors there is normal.

> > + nand_cleanup(chip);
> > +}
>

Thanks,
Miquèl