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

From: Miquel Raynal <[email-masked]>
Date: Mon, 24 Jul 2023 09:09:02 +0200

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